[RFC v2 00/20] efi_loader: more tightly integrate UEFI disks to driver model

[RFC v2 00/20] efi_loader: more tightly integrate UEFI disks to driver model

[AKASHI Takahiro]

# This is a kind of snapshot of my current work.
# I submit this to show my progress on this effort, and then
# I haven't addressed all the issues in this version.
# See the change history below.

The purpose of this RPC is to reignite the discussion about how UEFI
subystem would best be integrated into U-Boot driver model.
In the past, I poposed a couple of patch series, the latest one[1],
while Heinrich revealed his idea[2], and the approach taken here is
something between them, with a focus on block device handlings.

# The code is a PoC and not well tested yet.

Disks in UEFI world:
====================
In general in UEFI world, accessing to any device is performed through
a 'protocol' interface which are installed to (or associated with) the device's
UEFI handle (or an opaque pointer to UEFI object data). Protocols are
implemented by either the UEFI system itself or UEFI drivers.

For block IO's, it is a device which has EFI_BLOCK_IO_PROTOCOL (efi_disk
hereafter). Currently, every efi_disk may have one of two origins:

a.U-Boot's block devices or related partitions
  (lib/efi_loader/efi_disk.c)
b.UEFI objects which are implemented as a block device by UEFI drivers.
  (lib/efi_driver/efi_block_device.c)

All the efi_diskss as (a) will be enumelated and created only once at UEFI
subsystem initialization (efi_disk_register()), which is triggered by
first executing one of UEFI-related U-Boot commands, like "bootefi",
"setenv -e" or "efidebug".
EFI_BLOCK_IO_PROTOCOL is implemented by UEFI system using blk_desc(->ops)
in the corresponding udevice(UCLASS_BLK).

On the other hand, efi_disk as (b) will be created each time UEFI boot
services' connect_controller() is executed in UEFI app which, as a (device)
controller, gives the method to access the device's data,
ie. EFI_BLOCK_IO_PROTOCOL.

>>> more details >>>
Internally, connect_controller() search for UEFI driver that can support
this controller/protocol, 'efi_block' driver(UCLASS_EFI) in this case,
then calls the driver's 'bind' interface, which eventually installs
the controller's EFI_BLOCK_IO_PROTOCOL to efi_disk object.
'efi_block' driver also create a corresponding udevice(UCLASS_BLK) for
  * creating additional partitions efi_disk's, and
  * supporting a file system (EFI_SIMPLE_FILE_SYSTEM_PROTOCOL) on it.
<<< <<<

Issues:
=======
1. While an efi_disk represents a device equally for either a whole disk
   or a partition in UEFI world, the driver model treats only a whole
   disk as a real block device or udevice(UCLASS_BLK).

2. efi_disk holds and makes use of "blk_desc" data even though blk_desc
   in plat_data is supposed to be private and not to be accessed outside
   the driver model.
   # This issue, though, exists for all the implmenetation of U-Boot
   # file systems as well.

For efi_disk(a),
3. A block device can be enumelated dynamically by 'scanning' a device bus
   in U-Boot, but UEFI subsystem is not able to update efi_disks accordingly.
   For examples,
    => scsi rescan; efidebug devices
    => usb start; efidebug devices ... (A)
   (A) doesn't show any usb devices detected.

    => scsi rescan; efidebug boot add -b 0 TEST scsi 0:1 ...
    => scsi rescan ... (B)
    => bootefi bootmgr ... (C)
   (C) may de-reference a bogus blk_desc pointer which has been freed by (B).
   (Please note that "scsi rescan" removes all udevices/blk_desc and then
    re-create them even if nothing is changed on a bus.)

For efi_disk(b),
4. A "controller (handle)", combined with efi_block driver, has no
   corresponding udevice as a parent of efi_disks in DM tree, unlike,
   say, a scsi controller, even though it provides methods for block io
   operations.
5. There is no way supported to remove efi_disk's even after
   disconnect_controller() is called.


My approach in this RFC:
========================
Due to functional differences in semantics, it would be difficult
to identify "udevice" structure as a handle in UEFI world. Instead, we will
have to somehow maintain a relationship between a udevice and a handle.

1-1. add a dedicated uclass, UCLASS_PARTITION, for partitions
   Currently, the uclass for paritions is not a UCLASS_BLK.
   It can be possible to define partitions as UCLASS_BLK
   (with IF_TYPE_PARTION?), but
   I'm afraid that it may introduce some chaos since udevice(UCLASS_BLK)
   is tightly coupled with 'struct blk_desc' data which is still used
   as a "structure to a whole disk" in a lot of interfaces.
   (I hope that you understand what it means.)

   In DM tree, a UCLASS_PARTITON instance has a UCLASS_BLK parent:
   For instance,
       UCLASS_SCSI  --- UCLASS_BLK       --- UCLASS_PARTITION
			 (IF_TYPE_SCSI)        |
                          +- struct blk_desc   +- struct disk_part
			  +- scsi_blk_ops      +- blk_part_ops

1-2. create partition udevices in the context of device_probe() 
   part_init() is already called in blk_post_probe(). See the commit
   d0851c893706 ("blk: Call part_init() in the post_probe() method").
   Why not enumelate partitions as well in there.

2. add new block access interfaces, which takes a *udevice* as a target
   device, in U-Boot and use those functions to implement efi_disk
   operations (i.e. EFI_BLOCK_IO_PROTOCOL).

3-1. maintain a bi-directional link between a udevice and an efi_disk
   by adding
   - a UEFI handle pointer as a tag for a udevice
   - a udevice pointer in UEFI handle (in fact, in "struct efi_disk_obj")

3-2. synchronize the lifetime of efi_disk objects in UEFI world with
   the driver model using
   - event notification associated with device's probe/remove.

4. I have no answer to issue(4) and (5) yet.


Patchs:
=======
For easy understandings, patches may be categorized into seperate groups
of changes.

Patch#1-#10: DM: add device_probe() for later use of events
Patch#11-#12: DM: add new features (tag and event notification)
Patch#13-#16: UEFI: dynamically create/remove efi_disk's for a raw disk
  and its partitions
  For removal case, we may need more consideration since removing handles
  unconditionally may end up breaking integrity of handles
  (as some may still be held and referenced to by a UEFI app).
Patch#17-#18: UEFI: use udevice read/write interfaces
Patch#19-#20: UEFI: fix-up efi_driver, aligning with changes in DM integration


[1] https://lists.denx.de/pipermail/u-boot/2019-February/357923.html
[2] https://lists.denx.de/pipermail/u-boot/2021-June/452297.html


Change history:
===============
RFCv2 (Dec 10, 2021)
* rebased on 2022-rc3
* re-order and merge some related commits into ones
* call device_probe() in MMC (not bind, but) probe hook (patch#5)
* fix a wrong name of variable (patch#7)
* add patch#9
* invoke device_probe() for virtio devices (patch#10)
* add DM event notitication (from Simon) (patch#11)
* add DM tag support (patch#12)
* move UCLASS_PARTITION driver under disk/ (patch#13)
* create partition's dp using its parent's. This change is necessary
  in particular for 'efi_blk' efi_disk (patch#13)
* modify the code so that we will use new features like tags and
  event notification (patch#13,15,16,20)
* rename new functions from blk_read/write() to dev_read/write()
  (patch#17,18)
* isolate changes in efi_driver from the rest (in efi_loader) (patch#19)
* drop the previous patch#22 ("efi_selftest: block device: adjust dp
  for a test") due to the fix in patch#13

RFC (Nov 16, 2021)
* initial RFC

AKASHI Takahiro (19):
  part: call part_init() in blk_get_device_by_str() only for MMC
  blk: add a helper function, blk_probe_or_unbind()
  scsi: call device_probe() after scanning
  usb: storage: call device_probe() after scanning
  mmc: call device_probe() after scanning
  nvme: call device_probe() after scanning
  sata: call device_probe() after scanning
  block: ide: call device_probe() after scanning
  dm: fix an 'undefined' error in some macros
  virtio: call device_probe() in scanning
  dm: add tag support
  dm: disk: add UCLASS_PARTITION
  dm: blk: add a device-probe hook for scanning disk partitions
  efi_loader: disk: a helper function to create efi_disk objects from
    udevice
  efi_loader: disk: a helper function to delete efi_disk objects
  dm: disk: add read/write interfaces with udevice
  efi_loader: disk: use udevice instead of blk_desc
  efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI) devices
  efi_driver: align with efi_disk-dm integration

Simon Glass (1):
  dm: add event notification

 cmd/virtio.c                      |  21 +-
 common/Kconfig                    |  11 +
 common/Makefile                   |   2 +
 common/board_f.c                  |   2 +
 common/event.c                    | 103 ++++++++++
 common/log.c                      |   1 +
 common/usb_storage.c              |   4 +
 disk/Makefile                     |   3 +
 disk/disk-uclass.c                | 249 ++++++++++++++++++++++
 disk/part.c                       |   3 +-
 drivers/ata/dwc_ahsata.c          |   5 +
 drivers/ata/fsl_sata.c            |  11 +
 drivers/ata/sata_mv.c             |   5 +
 drivers/ata/sata_sil.c            |  12 ++
 drivers/block/blk-uclass.c        |  17 ++
 drivers/block/ide.c               |   4 +
 drivers/core/Makefile             |   2 +-
 drivers/core/device-remove.c      |   9 +
 drivers/core/device.c             |   9 +
 drivers/core/tag.c                | 162 +++++++++++++++
 drivers/mmc/mmc-uclass.c          |  13 ++
 drivers/nvme/nvme.c               |   4 +
 drivers/scsi/scsi.c               |   5 +
 include/asm-generic/global_data.h |   3 +
 include/blk.h                     |  12 ++
 include/dm/device-internal.h      |  10 +
 include/dm/device.h               |   8 +-
 include/dm/tag.h                  |  76 +++++++
 include/dm/uclass-id.h            |   1 +
 include/efi_loader.h              |   4 +-
 include/event.h                   | 105 ++++++++++
 include/event_internal.h          |  34 +++
 include/log.h                     |   2 +
 include/part.h                    |  18 ++
 lib/efi_driver/efi_block_device.c |  34 +--
 lib/efi_loader/Kconfig            |   2 +
 lib/efi_loader/efi_disk.c         | 329 ++++++++++++++++++++++--------
 lib/efi_loader/efi_setup.c        |  11 +-
 test/common/Makefile              |   1 +
 test/common/event.c               |  87 ++++++++
 test/test-main.c                  |   5 +
 41 files changed, 1274 insertions(+), 125 deletions(-)
 create mode 100644 common/event.c
 create mode 100644 disk/disk-uclass.c
 create mode 100644 drivers/core/tag.c
 create mode 100644 include/dm/tag.h
 create mode 100644 include/event.h
 create mode 100644 include/event_internal.h
 create mode 100644 test/common/event.c

-- 
2.33.0

[RFC v2 01/20] part: call part_init() in blk_get_device_by_str() only for MMC

[AKASHI Takahiro]

In blk_get_device_by_str(), the comment says: "Updates the partition table
for the specified hw partition."
Since hw partition is supported only on MMC, it makes no sense to do so
for other devices.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 disk/part.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/disk/part.c b/disk/part.c
index fe1ebd4adf7e..c0556a41e57f 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -427,7 +427,8 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str,
 	 * Always should be done, otherwise hw partition 0 will return stale
 	 * data after displaying a non-zero hw partition.
 	 */
-	part_init(*dev_desc);
+	if ((*dev_desc)->if_type == IF_TYPE_MMC)
+		part_init(*dev_desc);
 #endif
 
 cleanup:
-- 
2.33.0

[RFC v2 02/20] blk: add a helper function, blk_probe_or_unbind()

[AKASHI Takahiro]

This function will be commonly used in block device drivers
in the succeeding patches.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 drivers/block/blk-uclass.c | 13 +++++++++++++
 include/blk.h              | 12 ++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 83682dcc181a..f7ad90e8890f 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -670,6 +670,19 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
 	return 0;
 }
 
+int blk_probe_or_unbind(struct udevice *dev)
+{
+	int ret;
+
+	ret = device_probe(dev);
+	if (ret) {
+		debug("probing %s failed\n", dev->name);
+		device_unbind(dev);
+	}
+
+	return ret;
+}
+
 int blk_unbind_all(int if_type)
 {
 	struct uclass *uc;
diff --git a/include/blk.h b/include/blk.h
index f0cc7ca1a28c..ef79e7b27b0a 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -369,6 +369,18 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
 		       const char *name, int if_type, int devnum, int blksz,
 		       lbaint_t lba, struct udevice **devp);
 
+/**
+ * blk_probe_or_unbind() - Try to probe
+ *
+ * Try to probe the device, primarily for enumelating partitions.
+ * If it fails, the device itself is unbound since it means that it won't
+ * work any more.
+ *
+ * @dev:	The device to probe
+ * @return 0 if OK, -ve on error
+ */
+int blk_probe_or_unbind(struct udevice *dev);
+
 /**
  * blk_unbind_all() - Unbind all device of the given interface type
  *
-- 
2.33.0

[RFC v2 03/20] scsi: call device_probe() after scanning

[AKASHI Takahiro]

Every time a scsi bus/port is scanned and a new block device is detected,
we want to call device_probe() as it will give us a chance to run
additional post-processings for some purposes.

In particular, support for creating partitions on a device will be added.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 drivers/scsi/scsi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d93d24192853..9a763ea78bbd 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -595,6 +595,11 @@ static int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose)
 		ata_swap_buf_le16((u16 *)&bdesc->revision, sizeof(bd.revision) / 2);
 	}
 
+	ret = blk_probe_or_unbind(bdev);
+	if (ret < 0)
+		/* TODO: undo create */
+		return ret;
+
 	if (verbose) {
 		printf("  Device %d: ", bdesc->devnum);
 		dev_print(bdesc);
-- 
2.33.0

[RFC v2 04/20] usb: storage: call device_probe() after scanning

[AKASHI Takahiro]

Every time a usb bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run
additional post-processings for some purposes.

In particular, support for creating partitions on a device will be added.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 common/usb_storage.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/usb_storage.c b/common/usb_storage.c
index c9e2d7343ce2..291728f37e0a 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -239,6 +239,10 @@ static int usb_stor_probe_device(struct usb_device *udev)
 			if (ret)
 				return ret;
 		}
+
+		ret = blk_probe_or_unbind(dev);
+		if (ret)
+			return ret;
 	}
 #else
 	/* We don't have space to even probe if we hit the maximum */
-- 
2.33.0

[RFC v2 05/20] mmc: call device_probe() after scanning

[AKASHI Takahiro]

Every time a mmc bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run
additional post-processings for some purposes.

In particular, support for creating partitions on a device will be added.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 drivers/mmc/mmc-uclass.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 3ee92d03ca23..6c907b65fde7 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -418,6 +418,7 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
 	bdesc->part_type = cfg->part_type;
 	mmc->dev = dev;
 	mmc->user_speed_mode = MMC_MODES_END;
+
 	return 0;
 }
 
@@ -467,6 +468,18 @@ static int mmc_blk_probe(struct udevice *dev)
 		return ret;
 	}
 
+	ret = device_probe(dev);
+	if (ret) {
+		debug("Can't probe\n");
+
+		if (IS_ENABLED(CONFIG_MMC_UHS_SUPPORT) ||
+		    IS_ENABLED(CONFIG_MMC_HS200_SUPPORT) ||
+		    IS_ENABLED(CONFIG_MMC_HS400_SUPPORT))
+			mmc_deinit(mmc);
+
+		return ret;
+	}
+
 	return 0;
 }
 
-- 
2.33.0

[RFC v2 06/20] nvme: call device_probe() after scanning

[AKASHI Takahiro]

Every time a nvme bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run
additional post-processings for some purposes.

In particular, support for creating partitions on a device will be added.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 drivers/nvme/nvme.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 3c529a2fce22..120a0f05a591 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -910,6 +910,10 @@ static int nvme_probe(struct udevice *udev)
 					 -1, 512, 0, &ns_udev);
 		if (ret)
 			goto free_id;
+
+		ret = blk_probe_or_unbind(ns_udev);
+		if (ret)
+			goto free_id;
 	}
 
 	free(id);
-- 
2.33.0

[RFC v2 07/20] sata: call device_probe() after scanning

[AKASHI Takahiro]

Every time a sata bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run
additional post-processings for some purposes.

In particular, support for creating partitions on a device will be added.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 drivers/ata/dwc_ahsata.c |  5 +++++
 drivers/ata/fsl_sata.c   | 11 +++++++++++
 drivers/ata/sata_mv.c    |  5 +++++
 drivers/ata/sata_sil.c   | 12 ++++++++++++
 4 files changed, 33 insertions(+)

diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c
index 6d42548087b3..d9fd850c6fae 100644
--- a/drivers/ata/dwc_ahsata.c
+++ b/drivers/ata/dwc_ahsata.c
@@ -1026,6 +1026,11 @@ int dwc_ahsata_scan(struct udevice *dev)
 		return ret;
 	}
 
+	ret = blk_probe_or_unbind(dev);
+	if (ret < 0)
+		/* TODO: undo create */
+		return ret;
+
 	return 0;
 }
 
diff --git a/drivers/ata/fsl_sata.c b/drivers/ata/fsl_sata.c
index e44db0a37458..346e9298b4c5 100644
--- a/drivers/ata/fsl_sata.c
+++ b/drivers/ata/fsl_sata.c
@@ -982,6 +982,17 @@ static int fsl_ata_probe(struct udevice *dev)
 			failed_number++;
 			continue;
 		}
+
+		ret = device_probe(bdev);
+		if (ret < 0) {
+			debug("Can't probe\n");
+			ret = fsl_unbind_device(blk);
+			if (ret)
+				return ret;
+
+			failed_number++;
+			continue;
+		}
 	}
 
 	if (failed_number == nr_ports)
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 003222d47be6..60666d551434 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -1099,6 +1099,11 @@ static int sata_mv_probe(struct udevice *dev)
 			continue;
 		}
 
+		ret = blk_probe_or_unbind(bdev);
+		if (ret < 0)
+			/* TODO: undo create */
+			continue;
+
 		/* If we got here, the current SATA port was probed
 		 * successfully, so set the probe status to successful.
 		 */
diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c
index dda712f42cb2..295f7ca72303 100644
--- a/drivers/ata/sata_sil.c
+++ b/drivers/ata/sata_sil.c
@@ -864,6 +864,18 @@ static int sil_pci_probe(struct udevice *dev)
 			failed_number++;
 			continue;
 		}
+
+		ret = device_probe(bdev);
+		if (ret < 0) {
+			debug("Can't probe\n");
+			ret = sil_unbind_device(blk);
+			device_unbind(bdev);
+			if (ret)
+				return ret;
+
+			failed_number++;
+			continue;
+		}
 	}
 
 	if (failed_number == sata_info.maxport)
-- 
2.33.0

[RFC v2 08/20] block: ide: call device_probe() after scanning

[AKASHI Takahiro]

Every time an ide bus/port is scanned and a new device is detected,
we want to call device_probe() as it will give us a chance to run
additional post-processings for some purposes.

In particular, support for creating partitions on a device will be added.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 drivers/block/ide.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index c99076c6f45d..8071b5870931 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1151,6 +1151,10 @@ static int ide_probe(struct udevice *udev)
 						 blksz, size, &blk_dev);
 			if (ret)
 				return ret;
+
+			ret = blk_probe_or_unbind(blk_dev);
+			if (ret)
+				return ret;
 		}
 	}
 
-- 
2.33.0

[RFC v2 09/20] dm: fix an 'undefined' error in some macros

[AKASHI Takahiro]

Due to a non-existing parameter name in macro's, use of those macro's will
cause a compiler error of "undefined reference".
Unfortunately, dm test doesn't fail because a wrong name ("&dev", hence it
is accidentally a valid name in the context of a caller site) is passed on.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Fixes: f262d4ca4b2b ("dm: core: Add a way to read platdata for all
	child devices")
Fixes: 903e83ee8464 ("dm: core: Add a way to iterate through children,
	probing each")
---
 include/dm/device.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/dm/device.h b/include/dm/device.h
index 3028d002ab0d..f7900417066d 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -959,8 +959,8 @@ static inline bool device_is_on_pci_bus(const struct udevice *dev)
  * @parent: parent device to scan
  */
 #define device_foreach_child_of_to_plat(pos, parent)	\
-	for (int _ret = device_first_child_ofdata_err(parent, &dev); !_ret; \
-	     _ret = device_next_child_ofdata_err(&dev))
+	for (int _ret = device_first_child_ofdata_err(parent, &pos); !_ret; \
+	     _ret = device_next_child_ofdata_err(&pos))
 
 /**
  * device_foreach_child_probe() - iterate through children, probing them
@@ -976,8 +976,8 @@ static inline bool device_is_on_pci_bus(const struct udevice *dev)
  * @parent: parent device to scan
  */
 #define device_foreach_child_probe(pos, parent)	\
-	for (int _ret = device_first_child_err(parent, &dev); !_ret; \
-	     _ret = device_next_child_err(&dev))
+	for (int _ret = device_first_child_err(parent, &pos); !_ret; \
+	     _ret = device_next_child_err(&pos))
 
 /**
  * dm_scan_fdt_dev() - Bind child device in the device tree
-- 
2.33.0

[RFC v2 10/20] virtio: call device_probe() in scanning

[AKASHI Takahiro]

virtio_init() enumerates all the peripherals that are to be materialised
with udevices(UCLASS_VIRIO) and creates particular device instances
(UCLASS_BlK or whatever else) as children.
On the other hand, device_probe() won't be invoked against those resultant
udevices unlike other ordinary device drivers do in the driver model.

This is particularly incovenient when we want to add "event notification"
callback so that we will be able to automatically create all efi_disk
objects in a later patch.

With this patch applied, "virtio scan" will work in a similar way
to "scsi rescan", "usb start" or others in term of 'probe' semantics.

I didn't add this change to virtio_init() itself because this function
may be called in board_init_r() (indrectly in board_late_init())
before UEFI subsustem is initialized.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/virtio.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/cmd/virtio.c b/cmd/virtio.c
index 3dace5344f7e..ea3ed2e631e4 100644
--- a/cmd/virtio.c
+++ b/cmd/virtio.c
@@ -17,8 +17,25 @@ static int do_virtio(struct cmd_tbl *cmdtp, int flag, int argc,
 		     char *const argv[])
 {
 	if (argc == 2 && !strcmp(argv[1], "scan")) {
-		/* make sure all virtio devices are enumerated */
-		virtio_init();
+		/*
+		 * make sure all virtio devices are enumerated.
+		 * Do the same as virtio_init(), but also call
+		 * device_probe() for children (i.e. virtio devices)
+		 */
+		struct udevice *bus, *child;
+		int ret;
+
+		ret = uclass_first_device(UCLASS_VIRTIO, &bus);
+		if (ret)
+			return CMD_RET_FAILURE;
+
+		while (bus) {
+			device_foreach_child_probe(child, bus)
+				;
+			ret = uclass_next_device(&bus);
+			if (ret)
+				break;
+		}
 
 		return CMD_RET_SUCCESS;
 	}
-- 
2.33.0

[RFC v2 11/20] dm: add event notification

[AKASHI Takahiro]

From: Simon Glass <sjg@chromium.org>

This is a draft implementation of event notification mechanism from Simon.
Under this scheme, any U-Boot subsystem can register some kind of callback
function to a particular event (more event types will be added later) and
that function will be invoked once the event is fired.

As a first user, UEFI subsystem makes use of PROBE and REMOVE events so
that we will be able to automatically create/remove efi_disk objects
relying on associated block devices (UCLASS_BLK).

To run the test:
./u-boot -T -c "ut common test_event_probe"

Signed-off-by: Simon Glass <sjg@chromium.org>
[add REMOVE event; fix checkpatch warnings]
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 common/Kconfig                    |  11 ++++
 common/Makefile                   |   2 +
 common/board_f.c                  |   2 +
 common/event.c                    | 103 +++++++++++++++++++++++++++++
 common/log.c                      |   1 +
 drivers/core/device-remove.c      |   9 +++
 drivers/core/device.c             |   9 +++
 include/asm-generic/global_data.h |   3 +
 include/dm/device-internal.h      |  10 +++
 include/event.h                   | 105 ++++++++++++++++++++++++++++++
 include/event_internal.h          |  34 ++++++++++
 include/log.h                     |   2 +
 test/common/Makefile              |   1 +
 test/common/event.c               |  87 +++++++++++++++++++++++++
 test/test-main.c                  |   5 ++
 15 files changed, 384 insertions(+)
 create mode 100644 common/event.c
 create mode 100644 include/event.h
 create mode 100644 include/event_internal.h
 create mode 100644 test/common/event.c

diff --git a/common/Kconfig b/common/Kconfig
index fdcf4536d0f0..37b9276deeb2 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -482,6 +482,17 @@ config DISPLAY_BOARDINFO_LATE
 
 menu "Start-up hooks"
 
+config EVENT
+	bool "General-purpose event-handling mechanism"
+	default y if SANDBOX
+	help
+	  This enables sending and processing of events, to allow interested
+	  parties to be alerted when something happens. This is an attempt to
+	  step the flow of weak functions, hooks, functions in board_f.c
+	  and board_r.c and the Kconfig options below.
+
+	  See doc/develop/event.rst for more information.
+
 config ARCH_EARLY_INIT_R
 	bool "Call arch-specific init soon after relocation"
 	help
diff --git a/common/Makefile b/common/Makefile
index c500bcd7d8a0..bac7677e2f9e 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -90,6 +90,8 @@ obj-y += malloc_simple.o
 endif
 endif
 
+obj-$(CONFIG_EVENT) += event.o
+
 obj-$(CONFIG_$(SPL_TPL_)HASH) += hash.o
 obj-$(CONFIG_IO_TRACE) += iotrace.o
 obj-y += memsize.o
diff --git a/common/board_f.c b/common/board_f.c
index f7ea7c7a1e45..ca345f265004 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -19,6 +19,7 @@
 #include <dm.h>
 #include <env.h>
 #include <env_internal.h>
+#include <event.h>
 #include <fdtdec.h>
 #include <fs.h>
 #include <hang.h>
@@ -822,6 +823,7 @@ static const init_fnc_t init_sequence_f[] = {
 	initf_malloc,
 	log_init,
 	initf_bootstage,	/* uses its own timer, so does not need DM */
+	event_init,
 #ifdef CONFIG_BLOBLIST
 	bloblist_init,
 #endif
diff --git a/common/event.c b/common/event.c
new file mode 100644
index 000000000000..428628da44d6
--- /dev/null
+++ b/common/event.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Events provide a general-purpose way to react to / subscribe to changes
+ * within U-Boot
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#define LOG_CATEGORY	LOGC_EVENT
+
+#include <common.h>
+#include <event.h>
+#include <event_internal.h>
+#include <log.h>
+#include <malloc.h>
+#include <asm/global_data.h>
+#include <linux/list.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static void spy_free(struct event_spy *spy)
+{
+	list_del(&spy->sibling_node);
+}
+
+int event_register(const char *id, enum event_t type, event_handler_t func, void *ctx)
+{
+	struct event_state *state = gd->event_state;
+	struct event_spy *spy;
+
+	spy = malloc(sizeof(*spy));
+	if (!spy)
+		return log_msg_ret("alloc", -ENOMEM);
+
+	spy->id = id;
+	spy->type = type;
+	spy->func = func;
+	spy->ctx = ctx;
+	list_add_tail(&spy->sibling_node, &state->spy_head);
+
+	return 0;
+}
+
+int event_notify(enum event_t type, void *data, int size)
+{
+	struct event_state *state = gd->event_state;
+	struct event_spy *spy, *next;
+	struct event event;
+
+	event.type = type;
+	if (size > sizeof(event.data))
+		return log_msg_ret("size", -E2BIG);
+	memcpy(&event.data, data, size);
+	list_for_each_entry_safe(spy, next, &state->spy_head, sibling_node) {
+		if (spy->type == type) {
+			int ret;
+
+			log_debug("Sending event %x to spy '%s'\n", type,
+				  spy->id);
+			ret = spy->func(spy->ctx, &event);
+
+			/*
+			 * TODO: Handle various return codes to
+			 *
+			 * - claim an event (no others will see it)
+			 * - return an error from the event
+			 */
+			if (ret)
+				return log_msg_ret("spy", ret);
+		}
+	}
+
+	return 0;
+}
+
+int event_uninit(void)
+{
+	struct event_state *state = gd->event_state;
+	struct event_spy *spy, *next;
+
+	if (!state)
+		return 0;
+	list_for_each_entry_safe(spy, next, &state->spy_head, sibling_node)
+		spy_free(spy);
+
+	return 0;
+}
+
+int event_init(void)
+{
+	struct event_state *state;
+
+	state = malloc(sizeof(struct event_state));
+	if (!state)
+		return log_msg_ret("alloc", -ENOMEM);
+
+	INIT_LIST_HEAD(&state->spy_head);
+
+	gd->event_state = state;
+
+	return 0;
+}
diff --git a/common/log.c b/common/log.c
index 1aaa6c1527b9..aa2e780d2d7a 100644
--- a/common/log.c
+++ b/common/log.c
@@ -28,6 +28,7 @@ static const char *const log_cat_name[] = {
 	"devres",
 	"acpi",
 	"boot",
+	"event",
 };
 
 _Static_assert(ARRAY_SIZE(log_cat_name) == LOGC_COUNT - LOGC_NONE,
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index 11d3959d20f4..a63454f1b1d9 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -12,6 +12,7 @@
 
 #include <common.h>
 #include <errno.h>
+#include <event.h>
 #include <log.h>
 #include <malloc.h>
 #include <dm/device.h>
@@ -206,6 +207,10 @@ int device_remove(struct udevice *dev, uint flags)
 	if (!(dev_get_flags(dev) & DM_FLAG_ACTIVATED))
 		return 0;
 
+	ret = device_notify(dev, EVT_DM_PRE_REMOVE);
+	if (ret)
+		return ret;
+
 	/*
 	 * If the child returns EKEYREJECTED, continue. It just means that it
 	 * didn't match the flags.
@@ -255,6 +260,10 @@ int device_remove(struct udevice *dev, uint flags)
 
 	dev_bic_flags(dev, DM_FLAG_ACTIVATED);
 
+	ret = device_notify(dev, EVT_DM_POST_REMOVE);
+	if (ret)
+		goto err_remove;
+
 	return 0;
 
 err_remove:
diff --git a/drivers/core/device.c b/drivers/core/device.c
index efd07176e374..a5ba8a111b4c 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -10,6 +10,7 @@
 
 #include <common.h>
 #include <cpu_func.h>
+#include <event.h>
 #include <log.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
@@ -493,6 +494,10 @@ int device_probe(struct udevice *dev)
 	if (dev_get_flags(dev) & DM_FLAG_ACTIVATED)
 		return 0;
 
+	ret = device_notify(dev, EVT_DM_PRE_PROBE);
+	if (ret)
+		return ret;
+
 	drv = dev->driver;
 	assert(drv);
 
@@ -589,6 +594,10 @@ int device_probe(struct udevice *dev)
 	if (dev->parent && device_get_uclass_id(dev) == UCLASS_PINCTRL)
 		pinctrl_select_state(dev, "default");
 
+	ret = device_notify(dev, EVT_DM_POST_PROBE);
+	if (ret)
+		goto fail;
+
 	return 0;
 fail_uclass:
 	if (device_remove(dev, DM_REMOVE_NORMAL)) {
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 16fd305a65c1..771cda57a584 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -459,6 +459,9 @@ struct global_data {
 	 */
 	char *smbios_version;
 #endif
+#if CONFIG_IS_ENABLED(EVENT)
+	struct event_state *event_state;
+#endif
 };
 #ifndef DO_DEPS_ONLY
 static_assert(sizeof(struct global_data) == GD_SIZE);
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index e6b71cbfd2b6..1d0fd0b33c66 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -10,6 +10,7 @@
 #ifndef _DM_DEVICE_INTERNAL_H
 #define _DM_DEVICE_INTERNAL_H
 
+#include <event.h>
 #include <linker_lists.h>
 #include <dm/ofnode.h>
 
@@ -426,4 +427,13 @@ static inline void devres_release_all(struct udevice *dev)
 }
 
 #endif /* ! CONFIG_DEVRES */
+
+static inline int device_notify(const struct udevice *dev, enum event_t type)
+{
+#ifdef CONFIG_EVENT
+	return event_notify(type, &dev, sizeof(dev));
+#else
+	return 0;
+#endif
+}
 #endif
diff --git a/include/event.h b/include/event.h
new file mode 100644
index 000000000000..e2b74e6e62f0
--- /dev/null
+++ b/include/event.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Events provide a general-purpose way to react to / subscribe to changes
+ * within U-Boot
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#ifndef __event_h
+#define __event_h
+
+/**
+ * enum event_t - Types of events supported by U-Boot
+ *
+ * @EVT_DM_PRE_PROBE: Device is about to be probed
+ */
+enum event_t {
+	EVT_NONE,
+	EVT_TEST,
+
+	/* Events related to driver model */
+	EVT_DM_PRE_PROBE,
+	EVT_DM_POST_PROBE,
+	EVT_DM_PRE_REMOVE,
+	EVT_DM_POST_REMOVE,
+
+	EVT_COUNT
+};
+
+union event_data {
+	/**
+	 * struct event_data_test  - test data
+	 *
+	 * @signal: A value to update the state with
+	 */
+	struct event_data_test {
+		int signal;
+	} test;
+
+	/**
+	 * struct event_dm - driver model event
+	 *
+	 * @dev: Device this event relates to
+	 */
+	struct event_dm {
+		struct udevice *dev;
+	} dm;
+};
+
+/**
+ * struct event - an event that can be sent and received
+ *
+ * @type: Event type
+ * @data: Data for this particular event
+ */
+struct event {
+	enum event_t type;
+	union event_data data;
+};
+
+/** Function type for event handlers */
+typedef int (*event_handler_t)(void *ctx, struct event *event);
+
+/**
+ * event_register - register a new spy
+ *
+ * @id: Spy ID
+ * @type: Event type to subscribe to
+ * @func: Function to call when the event is sent
+ * @ctx: Context to pass to the function
+ * @return 0 if OK, -ve on erropr
+ */
+int event_register(const char *id, enum event_t type, event_handler_t func,
+		   void *ctx);
+
+/**
+ * event_notify() - notify spies about an event
+ *
+ * It is possible to pass in union event_data here but that may not be
+ * convenient if the data is elsewhere, or is one of the members of the union.
+ * So this uses a void * for @data, with a separate @size.
+ *
+ * @type: Event type
+ * @data: Event data to be sent (e.g. union_event_data)
+ * @size: Size of data in bytes
+ */
+int event_notify(enum event_t type, void *data, int size);
+
+#if CONFIG_IS_ENABLED(EVENT)
+int event_uninit(void);
+int event_init(void);
+#else
+static inline int event_uninit(void)
+{
+	return 0;
+}
+
+static inline int event_init(void)
+{
+	return 0;
+}
+#endif
+
+#endif
diff --git a/include/event_internal.h b/include/event_internal.h
new file mode 100644
index 000000000000..19308453f7b2
--- /dev/null
+++ b/include/event_internal.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Internal definitions for events
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#ifndef __event_internal_h
+#define __event_internal_h
+
+#include <linux/list.h>
+
+/**
+ * struct event_spy - a spy that watches for an event of a particular type
+ *
+ * @id: Spy ID
+ * @type: Event type to subscribe to
+ * @func: Function to call when the event is sent
+ * @ctx: Context to pass to the function
+ */
+struct event_spy {
+	struct list_head sibling_node;
+	const char *id;
+	enum event_t type;
+	event_handler_t func;
+	void *ctx;
+};
+
+struct event_state {
+	struct list_head spy_head;
+};
+
+#endif
diff --git a/include/log.h b/include/log.h
index e0e12ce1944e..7169fff7aa53 100644
--- a/include/log.h
+++ b/include/log.h
@@ -98,6 +98,8 @@ enum log_category_t {
 	LOGC_ACPI,
 	/** @LOGC_BOOT: Related to boot process / boot image processing */
 	LOGC_BOOT,
+	/** @LOGC_EVENT: Related to event and event handling */
+	LOGC_EVENT,
 	/** @LOGC_COUNT: Number of log categories */
 	LOGC_COUNT,
 	/** @LOGC_END: Sentinel value for lists of log categories */
diff --git a/test/common/Makefile b/test/common/Makefile
index 24c9145dccc8..9087788ba6a8 100644
--- a/test/common/Makefile
+++ b/test/common/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0+
 obj-y += cmd_ut_common.o
 obj-$(CONFIG_AUTOBOOT) += test_autoboot.o
+obj-$(CONFIG_EVENT) += event.o
diff --git a/test/common/event.c b/test/common/event.c
new file mode 100644
index 000000000000..ddce7400f269
--- /dev/null
+++ b/test/common/event.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Unit tests for event handling
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <event.h>
+#include <test/common.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+struct test_state {
+	struct udevice *dev;
+	int val;
+};
+
+static int h_adder(void *ctx, struct event *event)
+{
+	struct event_data_test *data = &event->data.test;
+	struct test_state *test_state = ctx;
+
+	test_state->val += data->signal;
+
+	return 0;
+}
+
+static int test_event_base(struct unit_test_state *uts)
+{
+	struct test_state state;
+	int signal;
+
+	state.val = 12;
+	ut_assertok(event_register("wibble", EVT_TEST, h_adder, &state));
+
+	signal = 17;
+
+	/* Check that the handler is called */
+	ut_assertok(event_notify(EVT_TEST, &signal, sizeof(signal)));
+	ut_asserteq(12 + 17, state.val);
+
+	return 0;
+}
+
+COMMON_TEST(test_event_base, 0);
+
+static int h_probe(void *ctx, struct event *event)
+{
+	struct test_state *test_state = ctx;
+
+	test_state->dev = event->data.dm.dev;
+	switch (event->type) {
+	case EVT_DM_PRE_PROBE:
+		test_state->val |= 1;
+		break;
+	case EVT_DM_POST_PROBE:
+		test_state->val |= 2;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int test_event_probe(struct unit_test_state *uts)
+{
+	struct test_state state;
+	struct udevice *dev;
+
+	state.val = 0;
+	ut_assertok(event_register("pre", EVT_DM_PRE_PROBE, h_probe, &state));
+	ut_assertok(event_register("post", EVT_DM_POST_PROBE, h_probe, &state));
+
+	/* Probe a device */
+	ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev));
+
+	/* Check that the handler is called */
+	ut_asserteq(3, state.val);
+
+	return 0;
+}
+
+COMMON_TEST(test_event_probe, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
diff --git a/test/test-main.c b/test/test-main.c
index 3cdf6849c57f..c3ccdf3fed83 100644
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -7,6 +7,7 @@
 #include <common.h>
 #include <console.h>
 #include <dm.h>
+#include <event.h>
 #include <dm/root.h>
 #include <dm/test.h>
 #include <dm/uclass-internal.h>
@@ -218,6 +219,9 @@ static int dm_test_restore(struct device_node *of_root)
  */
 static int test_pre_run(struct unit_test_state *uts, struct unit_test *test)
 {
+	gd->event_state = NULL;
+	ut_assertok(event_init());
+
 	if (test->flags & UT_TESTF_DM)
 		ut_assertok(dm_test_pre_run(uts));
 
@@ -260,6 +264,7 @@ static int test_post_run(struct unit_test_state *uts, struct unit_test *test)
 	ut_unsilence_console(uts);
 	if (test->flags & UT_TESTF_DM)
 		ut_assertok(dm_test_post_run(uts));
+	ut_assertok(event_uninit());
 
 	return 0;
 }
-- 
2.33.0

[RFC v2 12/20] dm: add tag support

[AKASHI Takahiro]

With dm-tag feature, any U-Boot subsystem is allowed to associate
arbitrary number of data with a particular udevice. This can been
see as expanding "struct udevice" without modifying the definition.

As a first user, UEFI subsystem makes use of tags to associate
an efi_disk object with a block device.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 drivers/core/Makefile |   2 +-
 drivers/core/tag.c    | 162 ++++++++++++++++++++++++++++++++++++++++++
 include/dm/tag.h      |  76 ++++++++++++++++++++
 3 files changed, 239 insertions(+), 1 deletion(-)
 create mode 100644 drivers/core/tag.c
 create mode 100644 include/dm/tag.h

diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index 5edd4e413576..3742e7574525 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -2,7 +2,7 @@
 #
 # Copyright (c) 2013 Google, Inc
 
-obj-y	+= device.o fdtaddr.o lists.o root.o uclass.o util.o
+obj-y	+= device.o fdtaddr.o lists.o root.o uclass.o util.o tag.o
 obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o
 obj-$(CONFIG_DEVRES) += devres.o
 obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE)	+= device-remove.o
diff --git a/drivers/core/tag.c b/drivers/core/tag.c
new file mode 100644
index 000000000000..562df3590c44
--- /dev/null
+++ b/drivers/core/tag.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021 Linaro Limited
+ *			Author: AKASHI Takahiro
+ */
+
+#include <malloc.h>
+#include <dm/tag.h>
+#include <linux/err.h>
+#include <linux/list.h>
+#include <linux/types.h>
+
+struct udevice;
+
+static LIST_HEAD(dmtag_list);
+
+/**
+ * dev_tag_set_ptr()
+ *
+ */
+int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr)
+{
+	struct dmtag_node *node;
+
+	if (!dev || tag > DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry(node, &dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag)
+			return -EEXIST;
+	}
+
+	node = calloc(sizeof(*node), 1);
+	if (!node)
+		return -ENOSPC;
+
+	node->dev = dev;
+	node->tag = tag;
+	node->ptr = ptr;
+	list_add_tail(&node->sibling, &dmtag_list);
+
+	return 0;
+}
+
+/**
+ * dev_tag_set_val()
+ *
+ */
+int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val)
+{
+	struct dmtag_node *node;
+
+	if (!dev || tag > DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry(node, &dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag)
+			return -EEXIST;
+	}
+
+	node = calloc(sizeof(*node), 1);
+	if (!node)
+		return -ENOSPC;
+
+	node->dev = dev;
+	node->tag = tag;
+	node->val = val;
+	list_add_tail(&node->sibling, &dmtag_list);
+
+	return 0;
+}
+
+/**
+ * dev_tag_get_ptr()
+ *
+ */
+int dev_tag_get_ptr(struct udevice *dev, enum dm_tag_t tag, void **ptrp)
+{
+	struct dmtag_node *node;
+
+	if (!dev || tag > DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry(node, &dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag) {
+			*ptrp = node->ptr;
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+/**
+ * dev_tag_get_val()
+ *
+ */
+int dev_tag_get_val(struct udevice *dev, enum dm_tag_t tag, ulong *valp)
+{
+	struct dmtag_node *node;
+
+	if (!dev || tag > DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry(node, &dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag) {
+			*valp = node->val;
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+/**
+ * dev_tag_del()
+ *
+ */
+int dev_tag_del(struct udevice *dev, enum dm_tag_t tag)
+{
+	struct dmtag_node *node, *tmp;
+
+	if (!dev || tag > DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry_safe(node, tmp, &dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag) {
+			list_del(&node->sibling);
+			free(node);
+
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+/**
+ * dev_tag_del_all()
+ *
+ */
+int dev_tag_del_all(struct udevice *dev)
+{
+	struct dmtag_node *node, *tmp;
+	bool found = false;
+
+	if (!dev)
+		return -EINVAL;
+
+	list_for_each_entry_safe(node, tmp, &dmtag_list, sibling) {
+		if (node->dev == dev) {
+			list_del(&node->sibling);
+			free(node);
+			found = true;
+		}
+	}
+
+	if (found)
+		return 0;
+
+	return -ENOENT;
+}
diff --git a/include/dm/tag.h b/include/dm/tag.h
new file mode 100644
index 000000000000..8f81b4f7ffaa
--- /dev/null
+++ b/include/dm/tag.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2021 Linaro Limited
+ *			Author: AKASHI Takahiro
+ */
+
+#ifndef _DM_TAG_H
+#define _DM_TAG_H
+
+#include <linux/list.h>
+#include <linux/types.h>
+
+struct udevice;
+
+enum dm_tag_t {
+	DM_TAG_EFI = 0,
+
+	DM_TAG_COUNT,
+};
+
+/**
+ * dmtag_node
+ *
+ * @sibling: List of dm-tag nodes
+ * @dev:     Associated udevice
+ * @tag:     Tag type
+ * @ptr:     Pointer as a value
+ * @val:     Value
+ */
+struct dmtag_node {
+	struct list_head sibling;
+	struct  udevice *dev;
+	enum dm_tag_t tag;
+	union {
+		void *ptr;
+		ulong val;
+	};
+};
+
+/**
+ * dev_tag_set_ptr()
+ *
+ */
+int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr);
+
+/**
+ * dev_tag_set_val()
+ *
+ */
+int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val);
+
+/**
+ * dev_tag_get_ptr()
+ *
+ */
+int dev_tag_get_ptr(struct udevice *dev, enum dm_tag_t tag, void **ptrp);
+
+/**
+ * dev_tag_get_val()
+ *
+ */
+int dev_tag_get_val(struct udevice *dev, enum dm_tag_t tag, ulong *valp);
+
+/**
+ * dev_tag_del()
+ *
+ */
+int dev_tag_del(struct udevice *dev, enum dm_tag_t tag);
+
+/**
+ * dev_tag_del_all()
+ *
+ */
+int dev_tag_del_all(struct udevice *dev);
+
+#endif /* _DM_TAG_H */
-- 
2.33.0

[RFC v2 13/20] dm: disk: add UCLASS_PARTITION

[AKASHI Takahiro]

With this new function, UCLASS_PARTITION devices will be created as
child nodes of UCLASS_BLK device.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 disk/Makefile          |   3 +
 disk/disk-uclass.c     | 153 +++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h |   1 +
 include/part.h         |  11 +++
 4 files changed, 168 insertions(+)
 create mode 100644 disk/disk-uclass.c

diff --git a/disk/Makefile b/disk/Makefile
index 6ce5a687b36c..ec37b74f5f40 100644
--- a/disk/Makefile
+++ b/disk/Makefile
@@ -6,6 +6,9 @@
 #ccflags-y += -DET_DEBUG -DDEBUG
 
 obj-$(CONFIG_PARTITIONS)	+= part.o
+ifdef CONFIG_$(SPL_)BLK
+obj-$(CONFIG_PARTITIONS) 	+= disk-uclass.o
+endif
 obj-$(CONFIG_$(SPL_)MAC_PARTITION)   += part_mac.o
 obj-$(CONFIG_$(SPL_)DOS_PARTITION)   += part_dos.o
 obj-$(CONFIG_$(SPL_)ISO_PARTITION)   += part_iso.o
diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c
new file mode 100644
index 000000000000..4918a2f72d1e
--- /dev/null
+++ b/disk/disk-uclass.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Software partition device (UCLASS_PARTITION)
+ *
+ *  Copyright (c) 2021 Linaro Limited
+ *			Author: AKASHI Takahiro
+ */
+
+#define LOG_CATEGORY UCLASS_PARTITION
+
+#include <blk.h>
+#include <dm.h>
+#include <log.h>
+#include <part.h>
+#include <vsprintf.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+
+int part_create_block_devices(struct udevice *blk_dev)
+{
+	int part, count;
+	struct blk_desc *desc = dev_get_uclass_plat(blk_dev);
+	struct disk_partition info;
+	struct disk_part *part_data;
+	char devname[32];
+	struct udevice *dev;
+	int ret;
+
+	if (!CONFIG_IS_ENABLED(PARTITIONS) ||
+	    !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))
+		return 0;
+
+	if (device_get_uclass_id(blk_dev) != UCLASS_BLK)
+		return 0;
+
+	/* Add devices for each partition */
+	for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
+		if (part_get_info(desc, part, &info))
+			continue;
+		snprintf(devname, sizeof(devname), "%s:%d", blk_dev->name,
+			 part);
+
+		ret = device_bind_driver(blk_dev, "blk_partition",
+					 strdup(devname), &dev);
+		if (ret)
+			return ret;
+
+		part_data = dev_get_uclass_plat(dev);
+		part_data->partnum = part;
+		part_data->gpt_part_info = info;
+		count++;
+
+		ret = device_probe(dev);
+		if (ret) {
+			debug("Can't probe\n");
+			count--;
+			device_unbind(dev);
+
+			continue;
+		}
+	}
+	debug("%s: %d partitions found in %s\n", __func__, count,
+	      blk_dev->name);
+
+	return 0;
+}
+
+static ulong blk_part_read(struct udevice *dev, lbaint_t start,
+			   lbaint_t blkcnt, void *buffer)
+{
+	struct udevice *parent;
+	struct disk_part *part;
+	const struct blk_ops *ops;
+
+	parent = dev_get_parent(dev);
+	ops = blk_get_ops(parent);
+	if (!ops->read)
+		return -ENOSYS;
+
+	part = dev_get_uclass_plat(dev);
+	if (start >= part->gpt_part_info.size)
+		return 0;
+
+	if ((start + blkcnt) > part->gpt_part_info.size)
+		blkcnt = part->gpt_part_info.size - start;
+	start += part->gpt_part_info.start;
+
+	return ops->read(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_write(struct udevice *dev, lbaint_t start,
+			    lbaint_t blkcnt, const void *buffer)
+{
+	struct udevice *parent;
+	struct disk_part *part;
+	const struct blk_ops *ops;
+
+	parent = dev_get_parent(dev);
+	ops = blk_get_ops(parent);
+	if (!ops->write)
+		return -ENOSYS;
+
+	part = dev_get_uclass_plat(dev);
+	if (start >= part->gpt_part_info.size)
+		return 0;
+
+	if ((start + blkcnt) > part->gpt_part_info.size)
+		blkcnt = part->gpt_part_info.size - start;
+	start += part->gpt_part_info.start;
+
+	return ops->write(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_erase(struct udevice *dev, lbaint_t start,
+			    lbaint_t blkcnt)
+{
+	struct udevice *parent;
+	struct disk_part *part;
+	const struct blk_ops *ops;
+
+	parent = dev_get_parent(dev);
+	ops = blk_get_ops(parent);
+	if (!ops->erase)
+		return -ENOSYS;
+
+	part = dev_get_uclass_plat(dev);
+	if (start >= part->gpt_part_info.size)
+		return 0;
+
+	if ((start + blkcnt) > part->gpt_part_info.size)
+		blkcnt = part->gpt_part_info.size - start;
+	start += part->gpt_part_info.start;
+
+	return ops->erase(parent, start, blkcnt);
+}
+
+static const struct blk_ops blk_part_ops = {
+	.read	= blk_part_read,
+	.write	= blk_part_write,
+	.erase	= blk_part_erase,
+};
+
+U_BOOT_DRIVER(blk_partition) = {
+	.name		= "blk_partition",
+	.id		= UCLASS_PARTITION,
+	.ops		= &blk_part_ops,
+};
+
+UCLASS_DRIVER(partition) = {
+	.id		= UCLASS_PARTITION,
+	.per_device_plat_auto	= sizeof(struct disk_part),
+	.name		= "partition",
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index fd139b9b2a08..e69de8568b9e 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -82,6 +82,7 @@ enum uclass_id {
 	UCLASS_P2SB,		/* (x86) Primary-to-Sideband Bus */
 	UCLASS_PANEL,		/* Display panel, such as an LCD */
 	UCLASS_PANEL_BACKLIGHT,	/* Backlight controller for panel */
+	UCLASS_PARTITION,	/* Logical disk partition device */
 	UCLASS_PCH,		/* x86 platform controller hub */
 	UCLASS_PCI,		/* PCI bus */
 	UCLASS_PCI_EP,		/* PCI endpoint device */
diff --git a/include/part.h b/include/part.h
index b66b07a1f04b..e492c2249019 100644
--- a/include/part.h
+++ b/include/part.h
@@ -253,6 +253,17 @@ void part_set_generic_name(const struct blk_desc *dev_desc,
 	int part_num, char *name);
 
 extern const struct block_drvr block_drvr[];
+
+struct udevice;
+/**
+ * part_create_block_devices - Create block devices for disk partitions
+ *
+ * Create UCLASS_PARTITION udevices for each of disk partitions in @parent
+ *
+ * @blk_dev:	Whole disk device
+ */
+int part_create_block_devices(struct udevice *blk_dev);
+
 #else
 static inline struct blk_desc *blk_get_dev(const char *ifname, int dev)
 { return NULL; }
-- 
2.33.0

[RFC v2 14/20] dm: blk: add a device-probe hook for scanning disk partitions

[AKASHI Takahiro]

Now that all the block device drivers have enable a probe hook, we will
call part_create_block_devices() to enumerate all the partitions and
create associated udevices when a block device is detected.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 drivers/block/blk-uclass.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index f7ad90e8890f..90687189e6d3 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -715,6 +715,10 @@ static int blk_post_probe(struct udevice *dev)
 		struct blk_desc *desc = dev_get_uclass_plat(dev);
 
 		part_init(desc);
+
+		if (desc->part_type != PART_TYPE_UNKNOWN &&
+		    part_create_block_devices(dev))
+			debug("*** creating partitions failed\n");
 	}
 
 	return 0;
-- 
2.33.0

[RFC v2 15/20] efi_loader: disk: a helper function to create efi_disk objects from udevice

[AKASHI Takahiro]

Add efi_disk_probe() function.
This function creates an efi_disk object for a raw disk device (UCLASS_BLK)
and additional objects for related partitions (UCLASS_PARTITION).

So this function is expected to be called through driver model's "probe"
interface every time one raw disk device is detected and activated.
We assume that partition devices (UCLASS_PARTITION) have been created
when this function is invoked.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/efi_loader.h       |   4 +-
 lib/efi_loader/Kconfig     |   2 +
 lib/efi_loader/efi_disk.c  | 206 ++++++++++++++++++++++++-------------
 lib/efi_loader/efi_setup.c |  11 +-
 4 files changed, 142 insertions(+), 81 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index d52e399841ba..a51095930efa 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -519,8 +519,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition,
 void efi_carve_out_dt_rsv(void *fdt);
 /* Called by bootefi to make console interface available */
 efi_status_t efi_console_register(void);
-/* Called by bootefi to make all disk storage accessible as EFI objects */
-efi_status_t efi_disk_register(void);
+/* Called by efi_init_obj_list() to initialize efi_disks */
+efi_status_t efi_disk_init(void);
 /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
 efi_status_t efi_rng_register(void);
 /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 700dc838ddb9..108c00343fce 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -11,6 +11,7 @@ config EFI_LOADER
 	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
 	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
 	depends on BLK
+	depends on EVENT
 	depends on DM_ETH || !NET
 	depends on !EFI_APP
 	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
@@ -41,6 +42,7 @@ config CMD_BOOTEFI_BOOTMGR
 
 config EFI_SETUP_EARLY
 	bool
+	default y
 
 choice
 	prompt "Store for non-volatile UEFI variables"
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 45127d176869..2941b0c3db47 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -10,6 +10,9 @@
 #include <common.h>
 #include <blk.h>
 #include <dm.h>
+#include <dm/device-internal.h>
+#include <dm/tag.h>
+#include <event.h>
 #include <efi_loader.h>
 #include <fs.h>
 #include <log.h>
@@ -487,103 +490,158 @@ error:
 	return ret;
 }
 
-/**
- * efi_disk_create_partitions() - create handles and protocols for partitions
+/*
+ * Create a handle for a whole raw disk
  *
- * Create handles and protocols for the partitions of a block device.
+ * @dev		uclass device (UCLASS_BLK)
  *
- * @parent:		handle of the parent disk
- * @desc:		block device
- * @if_typename:	interface type
- * @diskid:		device number
- * @pdevname:		device name
- * Return:		number of partitions created
+ * Create an efi_disk object which is associated with @dev.
+ * The type of @dev must be UCLASS_BLK.
+ *
+ * @return	0 on success, -1 otherwise
  */
-int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
-			       const char *if_typename, int diskid,
-			       const char *pdevname)
+static int efi_disk_create_raw(struct udevice *dev)
 {
-	int disks = 0;
-	char devname[32] = { 0 }; /* dp->str is u16[32] long */
-	int part;
-	struct efi_device_path *dp = NULL;
+	struct efi_disk_obj *disk;
+	struct blk_desc *desc;
+	const char *if_typename;
+	int diskid;
 	efi_status_t ret;
-	struct efi_handler *handler;
 
-	/* Get the device path of the parent */
-	ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
-	if (ret == EFI_SUCCESS)
-		dp = handler->protocol_interface;
-
-	/* Add devices for each partition */
-	for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
-		struct disk_partition info;
-
-		if (part_get_info(desc, part, &info))
-			continue;
-		snprintf(devname, sizeof(devname), "%s:%x", pdevname,
-			 part);
-		ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
-				       &info, part, NULL);
-		if (ret != EFI_SUCCESS) {
-			log_err("Adding partition %s failed\n", pdevname);
-			continue;
-		}
-		disks++;
+	desc = dev_get_uclass_plat(dev);
+	if_typename = blk_get_if_type_name(desc->if_type);
+	diskid = desc->devnum;
+
+	ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
+			       diskid, NULL, 0, &disk);
+	if (ret != EFI_SUCCESS) {
+		if (ret == EFI_NOT_READY)
+			log_notice("Disk %s not ready\n", dev->name);
+		else
+			log_err("Adding disk for %s failed\n", dev->name);
+
+		return -1;
+	}
+	disk->dev = dev;
+	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
+		efi_free_pool(disk->dp);
+		efi_delete_handle(&disk->header);
+
+		return -1;
 	}
 
-	return disks;
+	return 0;
 }
 
-/**
- * efi_disk_register() - register block devices
- *
- * U-Boot doesn't have a list of all online disk devices. So when running our
- * EFI payload, we scan through all of the potentially available ones and
- * store them in our object pool.
+/*
+ * Create a handle for a disk partition
  *
- * This function is called in efi_init_obj_list().
+ * @dev		uclass device (UCLASS_PARTITION)
  *
- * TODO(sjg@chromium.org): Actually with CONFIG_BLK, U-Boot does have this.
- * Consider converting the code to look up devices as needed. The EFI device
- * could be a child of the UCLASS_BLK block device, perhaps.
+ * Create an efi_disk object which is associated with @dev.
+ * The type of @dev must be UCLASS_PARTITION.
  *
- * Return:	status code
+ * @return	0 on success, -1 otherwise
  */
-efi_status_t efi_disk_register(void)
+static int efi_disk_create_part(struct udevice *dev)
 {
+	efi_handle_t parent;
+	struct blk_desc *desc;
+	const char *if_typename;
+	struct disk_part *part_data;
+	struct disk_partition *info;
+	unsigned int part;
+	int diskid;
+	struct efi_handler *handler;
+	struct efi_device_path *dp_parent;
 	struct efi_disk_obj *disk;
-	int disks = 0;
 	efi_status_t ret;
+
+	if (dev_tag_get_ptr(dev_get_parent(dev), DM_TAG_EFI, (void **)&parent))
+		return -1;
+
+	desc = dev_get_uclass_plat(dev_get_parent(dev));
+	if_typename = blk_get_if_type_name(desc->if_type);
+	diskid = desc->devnum;
+
+	part_data = dev_get_uclass_plat(dev);
+	part = part_data->partnum;
+	info = &part_data->gpt_part_info;
+
+	ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
+	if (ret != EFI_SUCCESS)
+		return -1;
+	dp_parent = (struct efi_device_path *)handler->protocol_interface;
+
+	ret = efi_disk_add_dev(parent, dp_parent, if_typename, desc, diskid,
+			       info, part, &disk);
+	if (ret != EFI_SUCCESS) {
+		log_err("Adding partition for %s failed\n", dev->name);
+		return -1;
+	}
+	disk->dev = dev;
+	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
+		efi_free_pool(disk->dp);
+		efi_delete_handle(&disk->header);
+
+		return -1;
+	}
+
+	return 0;
+}
+
+/*
+ * Create efi_disk objects for a block device
+ *
+ * @dev		uclass device (UCLASS_BLK)
+ *
+ * Create efi_disk objects for partitions as well as a raw disk
+ * which is associated with @dev.
+ * The type of @dev must be UCLASS_BLK.
+ * This function is expected to be called at EV_PM_POST_PROBE.
+ *
+ * @return	0 on success, -1 otherwise
+ */
+static int efi_disk_probe(void *ctx, struct event *event)
+{
 	struct udevice *dev;
+	enum uclass_id id;
+	struct udevice *child;
+	int ret;
 
-	for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
-	     uclass_next_device_check(&dev)) {
-		struct blk_desc *desc = dev_get_uclass_plat(dev);
-		const char *if_typename = blk_get_if_type_name(desc->if_type);
+	dev = event->data.dm.dev;
+	id = device_get_uclass_id(dev);
 
-		/* Add block device for the full device */
-		log_info("Scanning disk %s...\n", dev->name);
-		ret = efi_disk_add_dev(NULL, NULL, if_typename,
-					desc, desc->devnum, NULL, 0, &disk);
-		if (ret == EFI_NOT_READY) {
-			log_notice("Disk %s not ready\n", dev->name);
-			continue;
-		}
-		if (ret) {
-			log_err("ERROR: failure to add disk device %s, r = %lu\n",
-				dev->name, ret & ~EFI_ERROR_MASK);
-			continue;
-		}
-		disks++;
+	/* TODO: We won't support partitions in a partition */
+	if (id != UCLASS_BLK) {
+		if (id != UCLASS_PARTITION)
+			log_info("Not a block device: %s\n", dev->name);
+		return 0;
+	}
+
+	ret = efi_disk_create_raw(dev);
+	if (ret)
+		return -1;
 
-		/* Partitions show up as block devices in EFI */
-		disks += efi_disk_create_partitions(
-					&disk->header, desc, if_typename,
-					desc->devnum, dev->name);
+	device_foreach_child(child, dev) {
+		ret = efi_disk_create_part(child);
+		if (ret)
+			return -1;
 	}
 
-	log_info("Found %d disks\n", disks);
+	return 0;
+}
+
+efi_status_t efi_disk_init(void)
+{
+	int ret;
+
+	ret = event_register("efi_disk add", EVT_DM_POST_PROBE,
+			     efi_disk_probe, NULL);
+	if (ret) {
+		log_err("Event registration for efi_disk add failed\n");
+		return EFI_OUT_OF_RESOURCES;
+	}
 
 	return EFI_SUCCESS;
 }
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 1aba71cd9624..1c912b0157aa 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -200,11 +200,12 @@ efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
-#ifdef CONFIG_PARTITIONS
-	ret = efi_disk_register();
-	if (ret != EFI_SUCCESS)
-		goto out;
-#endif
+	if (IS_ENABLED(CONFIG_BLK)) {
+		ret = efi_disk_init();
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 	if (IS_ENABLED(CONFIG_EFI_RNG_PROTOCOL)) {
 		ret = efi_rng_register();
 		if (ret != EFI_SUCCESS)
-- 
2.33.0

[RFC v2 16/20] efi_loader: disk: a helper function to delete efi_disk objects

[AKASHI Takahiro]

This function is expected to be called, in particular from dm's pre_remove
hook, when associated block devices no longer exist.

Add efi_disk_remove() function.
This function removes an efi_disk object for a raw disk device (UCLASS_BLK)
and related objects for its partitions (UCLASS_PARTITION).

So this function is expected to be called through driver model's "remove"
interface every time a raw disk device is to be disconnected.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 lib/efi_loader/efi_disk.c | 88 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 2941b0c3db47..8dc7534278da 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -632,6 +632,87 @@ static int efi_disk_probe(void *ctx, struct event *event)
 	return 0;
 }
 
+/*
+ * Delete an efi_disk object for a whole raw disk
+ *
+ * @dev		uclass device (UCLASS_BLK)
+ *
+ * Delete an efi_disk object which is associated with @dev.
+ * The type of @dev must be UCLASS_BLK.
+ *
+ * @return	0 on success, -1 otherwise
+ */
+static int efi_disk_delete_raw(struct udevice *dev)
+{
+	efi_handle_t handle;
+	struct efi_disk_obj *diskobj;
+
+	if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
+		return -1;
+
+	diskobj = container_of(handle, struct efi_disk_obj, header);
+	efi_free_pool(diskobj->dp);
+
+	efi_delete_handle(handle);
+	dev_tag_del(dev, DM_TAG_EFI);
+
+	return 0;
+}
+
+/*
+ * Delete an efi_disk object for a disk partition
+ *
+ * @dev		uclass device (UCLASS_PARTITION)
+ *
+ * Delete an efi_disk object which is associated with @dev.
+ * The type of @dev must be UCLASS_PARTITION.
+ *
+ * @return	0 on success, -1 otherwise
+ */
+static int efi_disk_delete_part(struct udevice *dev)
+{
+	efi_handle_t handle;
+	struct efi_disk_obj *diskobj;
+
+	if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
+		return -1;
+
+	diskobj = container_of(handle, struct efi_disk_obj, header);
+
+	efi_free_pool(diskobj->dp);
+	efi_delete_handle(handle);
+	dev_tag_del(dev, DM_TAG_EFI);
+
+	return 0;
+}
+
+/*
+ * Delete an efi_disk object for a block device
+ *
+ * @dev		uclass device (UCLASS_BLK or UCLASS_PARTITION)
+ *
+ * Delete an efi_disk object which is associated with @dev.
+ * The type of @dev must be either UCLASS_BLK or UCLASS_PARTITION.
+ * This function is expected to be called at EV_PM_PRE_REMOVE.
+ *
+ * @return	0 on success, -1 otherwise
+ */
+static int efi_disk_remove(void *ctx, struct event *event)
+{
+	enum uclass_id id;
+	struct udevice *dev;
+
+	dev = event->data.dm.dev;
+	id = device_get_uclass_id(dev);
+
+	if (id == UCLASS_BLK)
+		return efi_disk_delete_raw(dev);
+	else if (id == UCLASS_PARTITION)
+		return efi_disk_delete_part(dev);
+	else
+		return 0;
+}
+
 efi_status_t efi_disk_init(void)
 {
 	int ret;
@@ -643,6 +724,13 @@ efi_status_t efi_disk_init(void)
 		return EFI_OUT_OF_RESOURCES;
 	}
 
+	ret = event_register("efi_disk del", EVT_DM_PRE_REMOVE,
+			     efi_disk_remove, NULL);
+	if (ret) {
+		log_err("Event registration for efi_disk del failed\n");
+		return EFI_OUT_OF_RESOURCES;
+	}
+
 	return EFI_SUCCESS;
 }
 
-- 
2.33.0

[RFC v2 17/20] dm: disk: add read/write interfaces with udevice

[AKASHI Takahiro]

In include/blk.h, Simon suggested:
===>
/*
 * These functions should take struct udevice instead of struct blk_desc,
 * but this is convenient for migration to driver model. Add a 'd' prefix
 * to the function operations, so that blk_read(), etc. can be reserved for
 * functions with the correct arguments.
 */
unsigned long blk_dread(struct blk_desc *block_dev, lbaint_t start,
                        lbaint_t blkcnt, void *buffer);
unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start,
                         lbaint_t blkcnt, const void *buffer);
unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
                         lbaint_t blkcnt);
<===

So new interfaces are provided with this patch.

They are expected to be used everywhere in U-Boot at the end.
The exceptions are block device drivers, partition drivers and efi_disk
which should know details of blk_desc structure.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 disk/disk-uclass.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++
 include/part.h     |  7 ++++
 2 files changed, 103 insertions(+)

diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c
index 4918a2f72d1e..9cadba37ab2d 100644
--- a/disk/disk-uclass.c
+++ b/disk/disk-uclass.c
@@ -146,6 +146,102 @@ U_BOOT_DRIVER(blk_partition) = {
 	.ops		= &blk_part_ops,
 };
 
+/*
+ * BLOCK IO APIs
+ */
+static struct blk_desc *dev_get_blk(struct udevice *dev)
+{
+	struct blk_desc *block_dev;
+
+	switch (device_get_uclass_id(dev)) {
+#if 0 /* FIXME */
+	case UCLASS_BLK:
+		block_dev = dev_get_uclass_plat(dev);
+		break;
+#endif
+	case UCLASS_PARTITION:
+		block_dev = dev_get_uclass_plat(dev_get_parent(dev));
+		break;
+	default:
+		block_dev = NULL;
+		break;
+	}
+
+	return block_dev;
+}
+
+unsigned long dev_read(struct udevice *dev, lbaint_t start,
+		       lbaint_t blkcnt, void *buffer)
+{
+	struct blk_desc *block_dev;
+	const struct blk_ops *ops;
+	struct disk_part *part;
+	lbaint_t start_in_disk;
+	ulong blks_read;
+
+	block_dev = dev_get_blk(dev);
+	if (!block_dev)
+		return -ENOSYS;
+
+	ops = blk_get_ops(dev);
+	if (!ops->read)
+		return -ENOSYS;
+
+	start_in_disk = start;
+	if (device_get_uclass_id(dev) == UCLASS_PARTITION) {
+		part = dev_get_uclass_plat(dev);
+		start_in_disk += part->gpt_part_info.start;
+	}
+
+	if (blkcache_read(block_dev->if_type, block_dev->devnum,
+			  start_in_disk, blkcnt, block_dev->blksz, buffer))
+		return blkcnt;
+	blks_read = ops->read(dev, start, blkcnt, buffer);
+	if (blks_read == blkcnt)
+		blkcache_fill(block_dev->if_type, block_dev->devnum,
+			      start_in_disk, blkcnt, block_dev->blksz, buffer);
+
+	return blks_read;
+}
+
+unsigned long dev_write(struct udevice *dev, lbaint_t start,
+			lbaint_t blkcnt, const void *buffer)
+{
+	struct blk_desc *block_dev;
+	const struct blk_ops *ops;
+
+	block_dev = dev_get_blk(dev);
+	if (!block_dev)
+		return -ENOSYS;
+
+	ops = blk_get_ops(dev);
+	if (!ops->write)
+		return -ENOSYS;
+
+	blkcache_invalidate(block_dev->if_type, block_dev->devnum);
+
+	return ops->write(dev, start, blkcnt, buffer);
+}
+
+unsigned long dev_erase(struct udevice *dev, lbaint_t start,
+			lbaint_t blkcnt)
+{
+	struct blk_desc *block_dev;
+	const struct blk_ops *ops;
+
+	block_dev = dev_get_blk(dev);
+	if (!block_dev)
+		return -ENOSYS;
+
+	ops = blk_get_ops(dev);
+	if (!ops->erase)
+		return -ENOSYS;
+
+	blkcache_invalidate(block_dev->if_type, block_dev->devnum);
+
+	return ops->erase(dev, start, blkcnt);
+}
+
 UCLASS_DRIVER(partition) = {
 	.id		= UCLASS_PARTITION,
 	.per_device_plat_auto	= sizeof(struct disk_part),
diff --git a/include/part.h b/include/part.h
index e492c2249019..1c564c6d6a3e 100644
--- a/include/part.h
+++ b/include/part.h
@@ -264,6 +264,13 @@ struct udevice;
  */
 int part_create_block_devices(struct udevice *blk_dev);
 
+unsigned long dev_read(struct udevice *dev, lbaint_t start,
+		       lbaint_t blkcnt, void *buffer);
+unsigned long dev_write(struct udevice *dev, lbaint_t start,
+			lbaint_t blkcnt, const void *buffer);
+unsigned long dev_erase(struct udevice *dev, lbaint_t start,
+			lbaint_t blkcnt);
+
 #else
 static inline struct blk_desc *blk_get_dev(const char *ifname, int dev)
 { return NULL; }
-- 
2.33.0

[RFC v2 18/20] efi_loader: disk: use udevice instead of blk_desc

[AKASHI Takahiro]

In most of all cases, we can avoid using blk_desc which is expected
to be private to udevice(UCLASS_BLK), that is, the data should not
be manupulated outside the device driver unless really needed.

Now efi_disk's internally use dev_read/write() interfaces.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 lib/efi_loader/efi_disk.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 8dc7534278da..8e33af76711f 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -48,7 +48,7 @@ struct efi_disk_obj {
 	unsigned int part;
 	struct efi_simple_file_system_protocol *volume;
 	lbaint_t offset;
-	struct blk_desc *desc;
+	struct udevice *dev; /* TODO: move it to efi_object */
 };
 
 /**
@@ -83,14 +83,12 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
 			void *buffer, enum efi_disk_direction direction)
 {
 	struct efi_disk_obj *diskobj;
-	struct blk_desc *desc;
 	int blksz;
 	int blocks;
 	unsigned long n;
 
 	diskobj = container_of(this, struct efi_disk_obj, ops);
-	desc = (struct blk_desc *) diskobj->desc;
-	blksz = desc->blksz;
+	blksz = diskobj->media.block_size;
 	blocks = buffer_size / blksz;
 	lba += diskobj->offset;
 
@@ -102,9 +100,9 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
 		return EFI_BAD_BUFFER_SIZE;
 
 	if (direction == EFI_DISK_READ)
-		n = blk_dread(desc, lba, blocks, buffer);
+		n = dev_read(diskobj->dev, lba, blocks, buffer);
 	else
-		n = blk_dwrite(desc, lba, blocks, buffer);
+		n = dev_write(diskobj->dev, lba, blocks, buffer);
 
 	/* We don't do interrupts, so check for timers cooperatively */
 	efi_timer_check();
@@ -446,7 +444,6 @@ static efi_status_t efi_disk_add_dev(
 	diskobj->ops = block_io_disk_template;
 	diskobj->ifname = if_typename;
 	diskobj->dev_index = dev_index;
-	diskobj->desc = desc;
 
 	/* Fill in EFI IO Media info (for read/write callbacks) */
 	diskobj->media.removable_media = desc->removable;
@@ -745,20 +742,22 @@ bool efi_disk_is_system_part(efi_handle_t handle)
 {
 	struct efi_handler *handler;
 	struct efi_disk_obj *diskobj;
-	struct disk_partition info;
+	struct udevice *dev;
+	struct disk_part *part;
 	efi_status_t ret;
-	int r;
 
 	/* check if this is a block device */
 	ret = efi_search_protocol(handle, &efi_block_io_guid, &handler);
 	if (ret != EFI_SUCCESS)
 		return false;
 
+	/* find a partition udevice */
 	diskobj = container_of(handle, struct efi_disk_obj, header);
-
-	r = part_get_info(diskobj->desc, diskobj->part, &info);
-	if (r)
+	dev = diskobj->dev;
+	if (!dev || dev->driver->id != UCLASS_PARTITION)
 		return false;
 
-	return !!(info.bootable & PART_EFI_SYSTEM_PARTITION);
+	part = dev_get_uclass_plat(dev);
+
+	return !!(part->gpt_part_info.bootable & PART_EFI_SYSTEM_PARTITION);
 }
-- 
2.33.0

[RFC v2 19/20] efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI) devices

[AKASHI Takahiro]

When we create an efi_disk device with an UEFI application using driver
binding protocol, the 'efi_driver' framework tries to create
a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to
calling a PROBE callback, efi_disk_probe().
In this case, however, we don't need to create another "efi_disk" device
as we already have this device instance.

So we should avoid recursively invoke further processing in the callback
function.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 lib/efi_loader/efi_disk.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 8e33af76711f..f37f8c1ab0f1 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -603,6 +603,7 @@ static int efi_disk_probe(void *ctx, struct event *event)
 {
 	struct udevice *dev;
 	enum uclass_id id;
+	struct blk_desc *desc;
 	struct udevice *child;
 	int ret;
 
@@ -616,9 +617,16 @@ static int efi_disk_probe(void *ctx, struct event *event)
 		return 0;
 	}
 
-	ret = efi_disk_create_raw(dev);
-	if (ret)
-		return -1;
+	/*
+	 * avoid creating duplicated objects now that efi_driver
+	 * has already created an efi_disk at this moment.
+	 */
+	desc = dev_get_uclass_plat(dev);
+	if (desc->if_type != IF_TYPE_EFI) {
+		ret = efi_disk_create_raw(dev);
+		if (ret)
+			return -1;
+	}
 
 	device_foreach_child(child, dev) {
 		ret = efi_disk_create_part(child);
@@ -642,13 +650,17 @@ static int efi_disk_probe(void *ctx, struct event *event)
 static int efi_disk_delete_raw(struct udevice *dev)
 {
 	efi_handle_t handle;
+	struct blk_desc *desc;
 	struct efi_disk_obj *diskobj;
 
 	if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
 		return -1;
 
-	diskobj = container_of(handle, struct efi_disk_obj, header);
-	efi_free_pool(diskobj->dp);
+	desc = dev_get_uclass_plat(dev);
+	if (desc->if_type != IF_TYPE_EFI) {
+		diskobj = container_of(handle, struct efi_disk_obj, header);
+		efi_free_pool(diskobj->dp);
+	}
 
 	efi_delete_handle(handle);
 	dev_tag_del(dev, DM_TAG_EFI);
-- 
2.33.0

[RFC v2 20/20] efi_driver: align with efi_disk-dm integration

[AKASHI Takahiro]

With DM-efi_disk integration, we don't need to explicitly call
efi_disk_create_partitions().

The only thing to do is to associate an efi_disk object to
the corresponding udevice as we skip most of processing in
efi_disk_probe() by the previous commit ("efi_loader: disk: not create
BLK device for BLK(IF_TYPE_EFI) devices").

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 lib/efi_driver/efi_block_device.c | 34 +++++++++----------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
index 0937e3595a43..28f672324991 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -35,6 +35,7 @@
 #include <malloc.h>
 #include <dm/device-internal.h>
 #include <dm/root.h>
+#include <dm/tag.h>
 
 /*
  * EFI attributes of the udevice handled by this driver.
@@ -106,25 +107,6 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	return blkcnt;
 }
 
-/**
- * Create partions for the block device.
- *
- * @handle:	EFI handle of the block device
- * @dev:	udevice of the block device
- * Return:	number of partitions created
- */
-static int efi_bl_bind_partitions(efi_handle_t handle, struct udevice *dev)
-{
-	struct blk_desc *desc;
-	const char *if_typename;
-
-	desc = dev_get_uclass_plat(dev);
-	if_typename = blk_get_if_type_name(desc->if_type);
-
-	return efi_disk_create_partitions(handle, desc, if_typename,
-					  desc->devnum, dev->name);
-}
-
 /**
  * Create a block device for a handle
  *
@@ -139,7 +121,6 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
 	char *name;
 	struct efi_object *obj = efi_search_obj(handle);
 	struct efi_block_io *io = interface;
-	int disks;
 	struct efi_blk_plat *plat;
 
 	EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io);
@@ -173,15 +154,20 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
 	plat->handle = handle;
 	plat->io = interface;
 
+	/*
+	 * FIXME: necessary because we won't do almost nothing in
+	 * efi_disk_create() when called from device_probe().
+	 */
+	ret = dev_tag_set_ptr(bdev, DM_TAG_EFI, handle);
+	if (ret)
+		/* FIXME: cleanup for bdev */
+		return ret;
+
 	ret = device_probe(bdev);
 	if (ret)
 		return ret;
 	EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name);
 
-	/* Create handles for the partions of the block device */
-	disks = efi_bl_bind_partitions(handle, bdev);
-	EFI_PRINT("Found %d partitions\n", disks);
-
 	return 0;
 }
 
-- 
2.33.0

Re: [RFC v2 02/20] blk: add a helper function, blk_probe_or_unbind()

[Simon Glass]

Hi Takahiro,

On Thu, 9 Dec 2021 at 23:59, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> This function will be commonly used in block device drivers
> in the succeeding patches.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  drivers/block/blk-uclass.c | 13 +++++++++++++
>  include/blk.h              | 12 ++++++++++++
>  2 files changed, 25 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

But please add a test for this.

>
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 83682dcc181a..f7ad90e8890f 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -670,6 +670,19 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
>         return 0;
>  }
>
> +int blk_probe_or_unbind(struct udevice *dev)
> +{
> +       int ret;
> +
> +       ret = device_probe(dev);
> +       if (ret) {
> +               debug("probing %s failed\n", dev->name);
> +               device_unbind(dev);
> +       }
> +
> +       return ret;
> +}
> +
>  int blk_unbind_all(int if_type)
>  {
>         struct uclass *uc;
> diff --git a/include/blk.h b/include/blk.h
> index f0cc7ca1a28c..ef79e7b27b0a 100644
> --- a/include/blk.h
> +++ b/include/blk.h
> @@ -369,6 +369,18 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
>                        const char *name, int if_type, int devnum, int blksz,
>                        lbaint_t lba, struct udevice **devp);
>
> +/**
> + * blk_probe_or_unbind() - Try to probe
> + *
> + * Try to probe the device, primarily for enumelating partitions.

enumerating


> + * If it fails, the device itself is unbound since it means that it won't
> + * work any more.
> + *
> + * @dev:       The device to probe
> + * @return 0 if OK, -ve on error
> + */
> +int blk_probe_or_unbind(struct udevice *dev);
> +
>  /**
>   * blk_unbind_all() - Unbind all device of the given interface type
>   *
> --
> 2.33.0
>

Re: [RFC v2 07/20] sata: call device_probe() after scanning

[Simon Glass]

On Fri, 10 Dec 2021 at 00:25, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Every time a sata bus/port is scanned and a new device is detected,
> we want to call device_probe() as it will give us a chance to run
> additional post-processings for some purposes.
>
> In particular, support for creating partitions on a device will be added.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  drivers/ata/dwc_ahsata.c |  5 +++++
>  drivers/ata/fsl_sata.c   | 11 +++++++++++
>  drivers/ata/sata_mv.c    |  5 +++++
>  drivers/ata/sata_sil.c   | 12 ++++++++++++
>  4 files changed, 33 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [RFC v2 09/20] dm: fix an 'undefined' error in some macros

[Simon Glass]

On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> Due to a non-existing parameter name in macro's, use of those macro's will
> cause a compiler error of "undefined reference".
> Unfortunately, dm test doesn't fail because a wrong name ("&dev", hence it
> is accidentally a valid name in the context of a caller site) is passed on.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Fixes: f262d4ca4b2b ("dm: core: Add a way to read platdata for all
>         child devices")
> Fixes: 903e83ee8464 ("dm: core: Add a way to iterate through children,
>         probing each")
> ---
>  include/dm/device.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [RFC v2 10/20] virtio: call device_probe() in scanning

[Simon Glass]

On Fri, 10 Dec 2021 at 00:00, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> virtio_init() enumerates all the peripherals that are to be materialised
> with udevices(UCLASS_VIRIO) and creates particular device instances
> (UCLASS_BlK or whatever else) as children.
> On the other hand, device_probe() won't be invoked against those resultant
> udevices unlike other ordinary device drivers do in the driver model.
>
> This is particularly incovenient when we want to add "event notification"
> callback so that we will be able to automatically create all efi_disk
> objects in a later patch.
>
> With this patch applied, "virtio scan" will work in a similar way
> to "scsi rescan", "usb start" or others in term of 'probe' semantics.
>
> I didn't add this change to virtio_init() itself because this function
> may be called in board_init_r() (indrectly in board_late_init())
> before UEFI subsustem is initialized.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  cmd/virtio.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [RFC v2 12/20] dm: add tag support

[Simon Glass]

Hi Takahiro,

On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> With dm-tag feature, any U-Boot subsystem is allowed to associate
> arbitrary number of data with a particular udevice. This can been
> see as expanding "struct udevice" without modifying the definition.
>
> As a first user, UEFI subsystem makes use of tags to associate
> an efi_disk object with a block device.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  drivers/core/Makefile |   2 +-
>  drivers/core/tag.c    | 162 ++++++++++++++++++++++++++++++++++++++++++
>  include/dm/tag.h      |  76 ++++++++++++++++++++
>  3 files changed, 239 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/core/tag.c
>  create mode 100644 include/dm/tag.h

Needs comments and a test, but looks good.

How does the list itself get inited?

Regards,
Simon

Re: [RFC v2 11/20] dm: add event notification

[Simon Glass]

Hi Takahiro,

On Fri, 10 Dec 2021 at 00:00, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> From: Simon Glass <sjg@chromium.org>
>
> This is a draft implementation of event notification mechanism from Simon.
> Under this scheme, any U-Boot subsystem can register some kind of callback
> function to a particular event (more event types will be added later) and
> that function will be invoked once the event is fired.
>
> As a first user, UEFI subsystem makes use of PROBE and REMOVE events so
> that we will be able to automatically create/remove efi_disk objects
> relying on associated block devices (UCLASS_BLK).
>
> To run the test:
> ./u-boot -T -c "ut common test_event_probe"
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> [add REMOVE event; fix checkpatch warnings]
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  common/Kconfig                    |  11 ++++
>  common/Makefile                   |   2 +
>  common/board_f.c                  |   2 +
>  common/event.c                    | 103 +++++++++++++++++++++++++++++
>  common/log.c                      |   1 +
>  drivers/core/device-remove.c      |   9 +++
>  drivers/core/device.c             |   9 +++
>  include/asm-generic/global_data.h |   3 +
>  include/dm/device-internal.h      |  10 +++
>  include/event.h                   | 105 ++++++++++++++++++++++++++++++
>  include/event_internal.h          |  34 ++++++++++
>  include/log.h                     |   2 +
>  test/common/Makefile              |   1 +
>  test/common/event.c               |  87 +++++++++++++++++++++++++
>  test/test-main.c                  |   5 ++
>  15 files changed, 384 insertions(+)
>  create mode 100644 common/event.c
>  create mode 100644 include/event.h
>  create mode 100644 include/event_internal.h
>  create mode 100644 test/common/event.c

Should we split this into a few patches for submission? Also needs docs.

Regards,
Simon

Re: [RFC v2 14/20] dm: blk: add a device-probe hook for scanning disk partitions

[Simon Glass]

On Thu, 9 Dec 2021 at 23:59, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> Now that all the block device drivers have enable a probe hook, we will
> call part_create_block_devices() to enumerate all the partitions and
> create associated udevices when a block device is detected.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  drivers/block/blk-uclass.c | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [RFC v2 18/20] efi_loader: disk: use udevice instead of blk_desc

[Simon Glass]

On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> In most of all cases, we can avoid using blk_desc which is expected
> to be private to udevice(UCLASS_BLK), that is, the data should not
> be manupulated outside the device driver unless really needed.
>
> Now efi_disk's internally use dev_read/write() interfaces.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  lib/efi_loader/efi_disk.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [RFC v2 19/20] efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI) devices

[Simon Glass]

On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> When we create an efi_disk device with an UEFI application using driver
> binding protocol, the 'efi_driver' framework tries to create
> a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to
> calling a PROBE callback, efi_disk_probe().
> In this case, however, we don't need to create another "efi_disk" device
> as we already have this device instance.
>
> So we should avoid recursively invoke further processing in the callback
> function.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  lib/efi_loader/efi_disk.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [RFC v2 20/20] efi_driver: align with efi_disk-dm integration

[Simon Glass]

On Fri, 10 Dec 2021 at 00:19, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> With DM-efi_disk integration, we don't need to explicitly call
> efi_disk_create_partitions().
>
> The only thing to do is to associate an efi_disk object to
> the corresponding udevice as we skip most of processing in
> efi_disk_probe() by the previous commit ("efi_loader: disk: not create
> BLK device for BLK(IF_TYPE_EFI) devices").
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  lib/efi_driver/efi_block_device.c | 34 +++++++++----------------------
>  1 file changed, 10 insertions(+), 24 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [RFC v2 02/20] blk: add a helper function, blk_probe_or_unbind()

[AKASHI Takahiro]

Hi Simon,

Thank you for your review on this series.

On Mon, Dec 13, 2021 at 05:51:40AM -0700, Simon Glass wrote:
> Hi Takahiro,
> 
> On Thu, 9 Dec 2021 at 23:59, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >
> > This function will be commonly used in block device drivers
> > in the succeeding patches.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  drivers/block/blk-uclass.c | 13 +++++++++++++
> >  include/blk.h              | 12 ++++++++++++
> >  2 files changed, 25 insertions(+)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> But please add a test for this.

Well, how can we test this simple function?
I can't simply imagine what the meaningful test scenario is.

> >
> > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> > index 83682dcc181a..f7ad90e8890f 100644
> > --- a/drivers/block/blk-uclass.c
> > +++ b/drivers/block/blk-uclass.c
> > @@ -670,6 +670,19 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
> >         return 0;
> >  }
> >
> > +int blk_probe_or_unbind(struct udevice *dev)
> > +{
> > +       int ret;
> > +
> > +       ret = device_probe(dev);
> > +       if (ret) {
> > +               debug("probing %s failed\n", dev->name);
> > +               device_unbind(dev);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> >  int blk_unbind_all(int if_type)
> >  {
> >         struct uclass *uc;
> > diff --git a/include/blk.h b/include/blk.h
> > index f0cc7ca1a28c..ef79e7b27b0a 100644
> > --- a/include/blk.h
> > +++ b/include/blk.h
> > @@ -369,6 +369,18 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
> >                        const char *name, int if_type, int devnum, int blksz,
> >                        lbaint_t lba, struct udevice **devp);
> >
> > +/**
> > + * blk_probe_or_unbind() - Try to probe
> > + *
> > + * Try to probe the device, primarily for enumelating partitions.
> 
> enumerating

Ah, OK.

-Takahiro Akashi

> 
> 
> > + * If it fails, the device itself is unbound since it means that it won't
> > + * work any more.
> > + *
> > + * @dev:       The device to probe
> > + * @return 0 if OK, -ve on error
> > + */
> > +int blk_probe_or_unbind(struct udevice *dev);
> > +
> >  /**
> >   * blk_unbind_all() - Unbind all device of the given interface type
> >   *
> > --
> > 2.33.0
> >

Re: [RFC v2 12/20] dm: add tag support

[AKASHI Takahiro]

On Mon, Dec 13, 2021 at 05:51:45AM -0700, Simon Glass wrote:
> Hi Takahiro,
> 
> On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >
> > With dm-tag feature, any U-Boot subsystem is allowed to associate
> > arbitrary number of data with a particular udevice. This can been
> > see as expanding "struct udevice" without modifying the definition.
> >
> > As a first user, UEFI subsystem makes use of tags to associate
> > an efi_disk object with a block device.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  drivers/core/Makefile |   2 +-
> >  drivers/core/tag.c    | 162 ++++++++++++++++++++++++++++++++++++++++++
> >  include/dm/tag.h      |  76 ++++++++++++++++++++
> >  3 files changed, 239 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/core/tag.c
> >  create mode 100644 include/dm/tag.h
> 
> Needs comments and a test, but looks good.
> 
> How does the list itself get inited?

Isn't LIST_HEAD(dmtag_list) enough?

-Takahiro Akashi


> Regards,
> Simon

Re: [RFC v2 11/20] dm: add event notification

[AKASHI Takahiro]

On Mon, Dec 13, 2021 at 05:51:46AM -0700, Simon Glass wrote:
> Hi Takahiro,
> 
> On Fri, 10 Dec 2021 at 00:00, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > From: Simon Glass <sjg@chromium.org>
> >
> > This is a draft implementation of event notification mechanism from Simon.
> > Under this scheme, any U-Boot subsystem can register some kind of callback
> > function to a particular event (more event types will be added later) and
> > that function will be invoked once the event is fired.
> >
> > As a first user, UEFI subsystem makes use of PROBE and REMOVE events so
> > that we will be able to automatically create/remove efi_disk objects
> > relying on associated block devices (UCLASS_BLK).
> >
> > To run the test:
> > ./u-boot -T -c "ut common test_event_probe"
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > [add REMOVE event; fix checkpatch warnings]
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  common/Kconfig                    |  11 ++++
> >  common/Makefile                   |   2 +
> >  common/board_f.c                  |   2 +
> >  common/event.c                    | 103 +++++++++++++++++++++++++++++
> >  common/log.c                      |   1 +
> >  drivers/core/device-remove.c      |   9 +++
> >  drivers/core/device.c             |   9 +++
> >  include/asm-generic/global_data.h |   3 +
> >  include/dm/device-internal.h      |  10 +++
> >  include/event.h                   | 105 ++++++++++++++++++++++++++++++
> >  include/event_internal.h          |  34 ++++++++++
> >  include/log.h                     |   2 +
> >  test/common/Makefile              |   1 +
> >  test/common/event.c               |  87 +++++++++++++++++++++++++
> >  test/test-main.c                  |   5 ++
> >  15 files changed, 384 insertions(+)
> >  create mode 100644 common/event.c
> >  create mode 100644 include/event.h
> >  create mode 100644 include/event_internal.h
> >  create mode 100644 test/common/event.c
> 
> Should we split this into a few patches for submission? Also needs docs.

I assumed that you were going to maintain this specific patch.

-Takahiro Akashi

> Regards,
> Simon

Re: [RFC v2 05/20] mmc: call device_probe() after scanning

[Heinrich Schuchardt]

On 12/10/21 07:49, AKASHI Takahiro wrote:
> Every time a mmc bus/port is scanned and a new device is detected,
> we want to call device_probe() as it will give us a chance to run
> additional post-processings for some purposes.
>
> In particular, support for creating partitions on a device will be added.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   drivers/mmc/mmc-uclass.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 3ee92d03ca23..6c907b65fde7 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -418,6 +418,7 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
>   	bdesc->part_type = cfg->part_type;
>   	mmc->dev = dev;
>   	mmc->user_speed_mode = MMC_MODES_END;
> +
>   	return 0;
>   }
>
> @@ -467,6 +468,18 @@ static int mmc_blk_probe(struct udevice *dev)
>   		return ret;
>   	}
>
> +	ret = device_probe(dev);
> +	if (ret) {
> +		debug("Can't probe\n");
> +
> +		if (IS_ENABLED(CONFIG_MMC_UHS_SUPPORT) ||
> +		    IS_ENABLED(CONFIG_MMC_HS200_SUPPORT) ||
> +		    IS_ENABLED(CONFIG_MMC_HS400_SUPPORT))

Not even sandbox_defconfig builds with your patch:

drivers/mmc/mmc-uclass.c: In function ‘mmc_blk_probe’:
drivers/mmc/mmc-uclass.c:478:25: warning: implicit declaration of
function ‘mmc_deinit’; did you mean ‘mmc_reinit’?
[-Wimplicit-function-declaration]
   478 |                         mmc_deinit(mmc);
       |                         ^~~~~~~~~~
       |                         mmc_reinit

Please, have a look at include/mmc.h:

790 #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT) || \
791     CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) || \
792     CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
793 int mmc_deinit(struct mmc *mmc);
794 #endif

mmc_deinit() may not be defined.

We should fix include/mmc.h first.

Best regards

Heinrich

> +			mmc_deinit(mmc);
> +
> +		return ret;
> +	}
> +
>   	return 0;
>   }
>

Re: [RFC v2 05/20] mmc: call device_probe() after scanning

[Heinrich Schuchardt]

On 12/18/21 10:48, Heinrich Schuchardt wrote:
> On 12/10/21 07:49, AKASHI Takahiro wrote:
>> Every time a mmc bus/port is scanned and a new device is detected,
>> we want to call device_probe() as it will give us a chance to run
>> additional post-processings for some purposes.
>>
>> In particular, support for creating partitions on a device will be added.
>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Cc: Jaehoon Chung <jh80.chung@samsung.com>, Peng Fan <peng.fan@nxp.com>

Please, use scripts/get_maintainers.pl to identify whom to send patches.

>> ---
>>   drivers/mmc/mmc-uclass.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
>> index 3ee92d03ca23..6c907b65fde7 100644
>> --- a/drivers/mmc/mmc-uclass.c
>> +++ b/drivers/mmc/mmc-uclass.c
>> @@ -418,6 +418,7 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc,
>> const struct mmc_config *cfg)
>>       bdesc->part_type = cfg->part_type;
>>       mmc->dev = dev;
>>       mmc->user_speed_mode = MMC_MODES_END;
>> +

Please avoid unrelated changes.

>>       return 0;
>>   }
>>
>> @@ -467,6 +468,18 @@ static int mmc_blk_probe(struct udevice *dev)
>>           return ret;
>>       }
>>
>> +    ret = device_probe(dev);
>> +    if (ret) {
>> +        debug("Can't probe\n");
>> +
>> +        if (IS_ENABLED(CONFIG_MMC_UHS_SUPPORT) ||
>> +            IS_ENABLED(CONFIG_MMC_HS200_SUPPORT) ||
>> +            IS_ENABLED(CONFIG_MMC_HS400_SUPPORT))
>
> Not even sandbox_defconfig builds with your patch:
>
> drivers/mmc/mmc-uclass.c: In function ‘mmc_blk_probe’:
> drivers/mmc/mmc-uclass.c:478:25: warning: implicit declaration of
> function ‘mmc_deinit’; did you mean ‘mmc_reinit’?
> [-Wimplicit-function-declaration]
>    478 |                         mmc_deinit(mmc);
>        |                         ^~~~~~~~~~
>        |                         mmc_reinit
>
> Please, have a look at include/mmc.h:
>
> 790 #if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT) || \
> 791     CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) || \
> 792     CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)
> 793 int mmc_deinit(struct mmc *mmc);
> 794 #endif
>
> mmc_deinit() may not be defined.
>
> We should fix include/mmc.h first.

See
[PATCH 1/1] mmc: unconditionally define mmc_deinit()
https://lists.denx.de/pipermail/u-boot/2021-December/469919.html

Best regards

Heinrich

>> +            mmc_deinit(mmc);
>> +
>> +        return ret;
>> +    }
>> +
>>       return 0;
>>   }
>>
>

Re: [RFC v2 02/20] blk: add a helper function, blk_probe_or_unbind()

[Heinrich Schuchardt]

On 12/14/21 08:01, AKASHI Takahiro wrote:
> Hi Simon,
>
> Thank you for your review on this series.
>
> On Mon, Dec 13, 2021 at 05:51:40AM -0700, Simon Glass wrote:
>> Hi Takahiro,
>>
>> On Thu, 9 Dec 2021 at 23:59, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>>>
>>> This function will be commonly used in block device drivers
>>> in the succeeding patches.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>> ---
>>>   drivers/block/blk-uclass.c | 13 +++++++++++++
>>>   include/blk.h              | 12 ++++++++++++
>>>   2 files changed, 25 insertions(+)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> But please add a test for this.
>
> Well, how can we test this simple function?
> I can't simply imagine what the meaningful test scenario is.
>
>>>
>>> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
>>> index 83682dcc181a..f7ad90e8890f 100644
>>> --- a/drivers/block/blk-uclass.c
>>> +++ b/drivers/block/blk-uclass.c
>>> @@ -670,6 +670,19 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
>>>          return 0;
>>>   }
>>>
>>> +int blk_probe_or_unbind(struct udevice *dev)
>>> +{
>>> +       int ret;
>>> +
>>> +       ret = device_probe(dev);
>>> +       if (ret) {
>>> +               debug("probing %s failed\n", dev->name);

%s/debug(/log_debug(/

>>> +               device_unbind(dev);
>>> +       }
>>> +
>>> +       return ret;
>>> +}
>>> +
>>>   int blk_unbind_all(int if_type)
>>>   {
>>>          struct uclass *uc;
>>> diff --git a/include/blk.h b/include/blk.h
>>> index f0cc7ca1a28c..ef79e7b27b0a 100644
>>> --- a/include/blk.h
>>> +++ b/include/blk.h
>>> @@ -369,6 +369,18 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
>>>                         const char *name, int if_type, int devnum, int blksz,
>>>                         lbaint_t lba, struct udevice **devp);
>>>
>>> +/**
>>> + * blk_probe_or_unbind() - Try to probe
>>> + *
>>> + * Try to probe the device, primarily for enumelating partitions.
>>
>> enumerating
>
> Ah, OK.
>
> -Takahiro Akashi
>
>>
>>
>>> + * If it fails, the device itself is unbound since it means that it won't
>>> + * work any more.
>>> + *
>>> + * @dev:       The device to probe
>>> + * @return 0 if OK, -ve on error

%s/@return/Return:/

Please, stick to Spinx style comments as documented in
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

Otherwise 'make htmldocs' will fail.

I will fix the remaining issues when merging.

Best regards

Heinrich

>>> + */
>>> +int blk_probe_or_unbind(struct udevice *dev);
>>> +
>>>   /**
>>>    * blk_unbind_all() - Unbind all device of the given interface type
>>>    *
>>> --
>>> 2.33.0
>>>

Re: [RFC v2 11/20] dm: add event notification

[Simon Glass]

Hi Takahiro,

On Tue, 14 Dec 2021 at 00:07, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> On Mon, Dec 13, 2021 at 05:51:46AM -0700, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Fri, 10 Dec 2021 at 00:00, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > From: Simon Glass <sjg@chromium.org>
> > >
> > > This is a draft implementation of event notification mechanism from Simon.
> > > Under this scheme, any U-Boot subsystem can register some kind of callback
> > > function to a particular event (more event types will be added later) and
> > > that function will be invoked once the event is fired.
> > >
> > > As a first user, UEFI subsystem makes use of PROBE and REMOVE events so
> > > that we will be able to automatically create/remove efi_disk objects
> > > relying on associated block devices (UCLASS_BLK).
> > >
> > > To run the test:
> > > ./u-boot -T -c "ut common test_event_probe"
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > [add REMOVE event; fix checkpatch warnings]
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > ---
> > >  common/Kconfig                    |  11 ++++
> > >  common/Makefile                   |   2 +
> > >  common/board_f.c                  |   2 +
> > >  common/event.c                    | 103 +++++++++++++++++++++++++++++
> > >  common/log.c                      |   1 +
> > >  drivers/core/device-remove.c      |   9 +++
> > >  drivers/core/device.c             |   9 +++
> > >  include/asm-generic/global_data.h |   3 +
> > >  include/dm/device-internal.h      |  10 +++
> > >  include/event.h                   | 105 ++++++++++++++++++++++++++++++
> > >  include/event_internal.h          |  34 ++++++++++
> > >  include/log.h                     |   2 +
> > >  test/common/Makefile              |   1 +
> > >  test/common/event.c               |  87 +++++++++++++++++++++++++
> > >  test/test-main.c                  |   5 ++
> > >  15 files changed, 384 insertions(+)
> > >  create mode 100644 common/event.c
> > >  create mode 100644 include/event.h
> > >  create mode 100644 include/event_internal.h
> > >  create mode 100644 test/common/event.c
> >
> > Should we split this into a few patches for submission? Also needs docs.
>
> I assumed that you were going to maintain this specific patch.

Ah yes, good point :-)

I will give it a crack.

Regards,
Simon

Re: [RFC v2 02/20] blk: add a helper function, blk_probe_or_unbind()

[AKASHI Takahiro]

On Sat, Dec 18, 2021 at 11:55:41AM +0100, Heinrich Schuchardt wrote:
> On 12/14/21 08:01, AKASHI Takahiro wrote:
> > Hi Simon,
> > 
> > Thank you for your review on this series.
> > 
> > On Mon, Dec 13, 2021 at 05:51:40AM -0700, Simon Glass wrote:
> > > Hi Takahiro,
> > > 
> > > On Thu, 9 Dec 2021 at 23:59, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > > 
> > > > This function will be commonly used in block device drivers
> > > > in the succeeding patches.
> > > > 
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > ---
> > > >   drivers/block/blk-uclass.c | 13 +++++++++++++
> > > >   include/blk.h              | 12 ++++++++++++
> > > >   2 files changed, 25 insertions(+)
> > > 
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > 
> > > But please add a test for this.
> > 
> > Well, how can we test this simple function?
> > I can't simply imagine what the meaningful test scenario is.
> > 
> > > > 
> > > > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> > > > index 83682dcc181a..f7ad90e8890f 100644
> > > > --- a/drivers/block/blk-uclass.c
> > > > +++ b/drivers/block/blk-uclass.c
> > > > @@ -670,6 +670,19 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
> > > >          return 0;
> > > >   }
> > > > 
> > > > +int blk_probe_or_unbind(struct udevice *dev)
> > > > +{
> > > > +       int ret;
> > > > +
> > > > +       ret = device_probe(dev);
> > > > +       if (ret) {
> > > > +               debug("probing %s failed\n", dev->name);
> 
> %s/debug(/log_debug(/

I used "debug()" here as it is commonly used in this file(blk-uclass.c).

> > > > +               device_unbind(dev);
> > > > +       }
> > > > +
> > > > +       return ret;
> > > > +}
> > > > +
> > > >   int blk_unbind_all(int if_type)
> > > >   {
> > > >          struct uclass *uc;
> > > > diff --git a/include/blk.h b/include/blk.h
> > > > index f0cc7ca1a28c..ef79e7b27b0a 100644
> > > > --- a/include/blk.h
> > > > +++ b/include/blk.h
> > > > @@ -369,6 +369,18 @@ int blk_create_devicef(struct udevice *parent, const char *drv_name,
> > > >                         const char *name, int if_type, int devnum, int blksz,
> > > >                         lbaint_t lba, struct udevice **devp);
> > > > 
> > > > +/**
> > > > + * blk_probe_or_unbind() - Try to probe
> > > > + *
> > > > + * Try to probe the device, primarily for enumelating partitions.
> > > 
> > > enumerating
> > 
> > Ah, OK.
> > 
> > -Takahiro Akashi
> > 
> > > 
> > > 
> > > > + * If it fails, the device itself is unbound since it means that it won't
> > > > + * work any more.
> > > > + *
> > > > + * @dev:       The device to probe
> > > > + * @return 0 if OK, -ve on error
> 
> %s/@return/Return:/

I used "@return" here as it is commonly used in this file(blk.h).

> Please, stick to Spinx style comments as documented in
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation
> 
> Otherwise 'make htmldocs' will fail.

If so, why don't other uses of "@return" cause failures as well?

> I will fix the remaining issues when merging.

Thank you.
We'd better fix all the occurrences in those files at the same time
to avoid possible confusion.

-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> > > > + */
> > > > +int blk_probe_or_unbind(struct udevice *dev);
> > > > +
> > > >   /**
> > > >    * blk_unbind_all() - Unbind all device of the given interface type
> > > >    *
> > > > --
> > > > 2.33.0
> > > > 
> 

Re: [RFC v2 12/20] dm: add tag support

[Simon Glass]

Hi Takahiro,

On Tue, 14 Dec 2021 at 00:05, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> On Mon, Dec 13, 2021 at 05:51:45AM -0700, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > >
> > > With dm-tag feature, any U-Boot subsystem is allowed to associate
> > > arbitrary number of data with a particular udevice. This can been
> > > see as expanding "struct udevice" without modifying the definition.
> > >
> > > As a first user, UEFI subsystem makes use of tags to associate
> > > an efi_disk object with a block device.
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > ---
> > >  drivers/core/Makefile |   2 +-
> > >  drivers/core/tag.c    | 162 ++++++++++++++++++++++++++++++++++++++++++
> > >  include/dm/tag.h      |  76 ++++++++++++++++++++
> > >  3 files changed, 239 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/core/tag.c
> > >  create mode 100644 include/dm/tag.h
> >
> > Needs comments and a test, but looks good.
> >
> > How does the list itself get inited?
>
> Isn't LIST_HEAD(dmtag_list) enough?

Yes but that is a BSS var so cannot be used in SPL or before
relocation. How about moving it to global_data?

Regards,
Simon

Re: [RFC v2 15/20] efi_loader: disk: a helper function to create efi_disk objects from udevice

[Heinrich Schuchardt]

On 12/10/21 07:49, AKASHI Takahiro wrote:
> Add efi_disk_probe() function.
> This function creates an efi_disk object for a raw disk device (UCLASS_BLK)
> and additional objects for related partitions (UCLASS_PARTITION).
>
> So this function is expected to be called through driver model's "probe"
> interface every time one raw disk device is detected and activated.
> We assume that partition devices (UCLASS_PARTITION) have been created
> when this function is invoked.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   include/efi_loader.h       |   4 +-
>   lib/efi_loader/Kconfig     |   2 +
>   lib/efi_loader/efi_disk.c  | 206 ++++++++++++++++++++++++-------------
>   lib/efi_loader/efi_setup.c |  11 +-
>   4 files changed, 142 insertions(+), 81 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index d52e399841ba..a51095930efa 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -519,8 +519,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition,
>   void efi_carve_out_dt_rsv(void *fdt);
>   /* Called by bootefi to make console interface available */
>   efi_status_t efi_console_register(void);
> -/* Called by bootefi to make all disk storage accessible as EFI objects */
> -efi_status_t efi_disk_register(void);
> +/* Called by efi_init_obj_list() to initialize efi_disks */
> +efi_status_t efi_disk_init(void);
>   /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
>   efi_status_t efi_rng_register(void);
>   /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 700dc838ddb9..108c00343fce 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -11,6 +11,7 @@ config EFI_LOADER
>   	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
>   	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
>   	depends on BLK
> +	depends on EVENT
>   	depends on DM_ETH || !NET
>   	depends on !EFI_APP
>   	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
> @@ -41,6 +42,7 @@ config CMD_BOOTEFI_BOOTMGR
>
>   config EFI_SETUP_EARLY
>   	bool
> +	default y
>
>   choice
>   	prompt "Store for non-volatile UEFI variables"
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 45127d176869..2941b0c3db47 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -10,6 +10,9 @@
>   #include <common.h>
>   #include <blk.h>
>   #include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/tag.h>
> +#include <event.h>
>   #include <efi_loader.h>
>   #include <fs.h>
>   #include <log.h>
> @@ -487,103 +490,158 @@ error:
>   	return ret;
>   }
>
> -/**
> - * efi_disk_create_partitions() - create handles and protocols for partitions
> +/*
> + * Create a handle for a whole raw disk
>    *
> - * Create handles and protocols for the partitions of a block device.
> + * @dev		uclass device (UCLASS_BLK)
>    *
> - * @parent:		handle of the parent disk
> - * @desc:		block device
> - * @if_typename:	interface type
> - * @diskid:		device number
> - * @pdevname:		device name
> - * Return:		number of partitions created
> + * Create an efi_disk object which is associated with @dev.
> + * The type of @dev must be UCLASS_BLK.
> + *
> + * @return	0 on success, -1 otherwise
>    */
> -int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
> -			       const char *if_typename, int diskid,
> -			       const char *pdevname)
> +static int efi_disk_create_raw(struct udevice *dev)
>   {
> -	int disks = 0;
> -	char devname[32] = { 0 }; /* dp->str is u16[32] long */
> -	int part;
> -	struct efi_device_path *dp = NULL;
> +	struct efi_disk_obj *disk;
> +	struct blk_desc *desc;
> +	const char *if_typename;
> +	int diskid;
>   	efi_status_t ret;
> -	struct efi_handler *handler;
>
> -	/* Get the device path of the parent */
> -	ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
> -	if (ret == EFI_SUCCESS)
> -		dp = handler->protocol_interface;
> -
> -	/* Add devices for each partition */
> -	for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
> -		struct disk_partition info;
> -
> -		if (part_get_info(desc, part, &info))
> -			continue;
> -		snprintf(devname, sizeof(devname), "%s:%x", pdevname,
> -			 part);
> -		ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
> -				       &info, part, NULL);
> -		if (ret != EFI_SUCCESS) {
> -			log_err("Adding partition %s failed\n", pdevname);
> -			continue;
> -		}
> -		disks++;
> +	desc = dev_get_uclass_plat(dev);
> +	if_typename = blk_get_if_type_name(desc->if_type);
> +	diskid = desc->devnum;
> +
> +	ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
> +			       diskid, NULL, 0, &disk);
> +	if (ret != EFI_SUCCESS) {
> +		if (ret == EFI_NOT_READY)
> +			log_notice("Disk %s not ready\n", dev->name);
> +		else
> +			log_err("Adding disk for %s failed\n", dev->name);
> +
> +		return -1;
> +	}
> +	disk->dev = dev;
> +	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
> +		efi_free_pool(disk->dp);
> +		efi_delete_handle(&disk->header);
> +
> +		return -1;
>   	}
>
> -	return disks;
> +	return 0;
>   }
>
> -/**
> - * efi_disk_register() - register block devices
> - *
> - * U-Boot doesn't have a list of all online disk devices. So when running our
> - * EFI payload, we scan through all of the potentially available ones and
> - * store them in our object pool.
> +/*
> + * Create a handle for a disk partition
>    *
> - * This function is called in efi_init_obj_list().
> + * @dev		uclass device (UCLASS_PARTITION)
>    *
> - * TODO(sjg@chromium.org): Actually with CONFIG_BLK, U-Boot does have this.
> - * Consider converting the code to look up devices as needed. The EFI device
> - * could be a child of the UCLASS_BLK block device, perhaps.
> + * Create an efi_disk object which is associated with @dev.
> + * The type of @dev must be UCLASS_PARTITION.
>    *
> - * Return:	status code
> + * @return	0 on success, -1 otherwise
>    */
> -efi_status_t efi_disk_register(void)
> +static int efi_disk_create_part(struct udevice *dev)
>   {
> +	efi_handle_t parent;
> +	struct blk_desc *desc;
> +	const char *if_typename;
> +	struct disk_part *part_data;
> +	struct disk_partition *info;
> +	unsigned int part;
> +	int diskid;
> +	struct efi_handler *handler;
> +	struct efi_device_path *dp_parent;
>   	struct efi_disk_obj *disk;
> -	int disks = 0;
>   	efi_status_t ret;
> +
> +	if (dev_tag_get_ptr(dev_get_parent(dev), DM_TAG_EFI, (void **)&parent))
> +		return -1;
> +
> +	desc = dev_get_uclass_plat(dev_get_parent(dev));
> +	if_typename = blk_get_if_type_name(desc->if_type);
> +	diskid = desc->devnum;
> +
> +	part_data = dev_get_uclass_plat(dev);
> +	part = part_data->partnum;
> +	info = &part_data->gpt_part_info;
> +
> +	ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
> +	if (ret != EFI_SUCCESS)
> +		return -1;
> +	dp_parent = (struct efi_device_path *)handler->protocol_interface;
> +
> +	ret = efi_disk_add_dev(parent, dp_parent, if_typename, desc, diskid,
> +			       info, part, &disk);
> +	if (ret != EFI_SUCCESS) {
> +		log_err("Adding partition for %s failed\n", dev->name);
> +		return -1;
> +	}
> +	disk->dev = dev;
> +	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
> +		efi_free_pool(disk->dp);
> +		efi_delete_handle(&disk->header);
> +
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Create efi_disk objects for a block device
> + *
> + * @dev		uclass device (UCLASS_BLK)
> + *
> + * Create efi_disk objects for partitions as well as a raw disk
> + * which is associated with @dev.
> + * The type of @dev must be UCLASS_BLK.
> + * This function is expected to be called at EV_PM_POST_PROBE.
> + *
> + * @return	0 on success, -1 otherwise
> + */
> +static int efi_disk_probe(void *ctx, struct event *event)
> +{
>   	struct udevice *dev;
> +	enum uclass_id id;
> +	struct udevice *child;
> +	int ret;
>
> -	for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
> -	     uclass_next_device_check(&dev)) {
> -		struct blk_desc *desc = dev_get_uclass_plat(dev);
> -		const char *if_typename = blk_get_if_type_name(desc->if_type);
> +	dev = event->data.dm.dev;
> +	id = device_get_uclass_id(dev);
>
> -		/* Add block device for the full device */
> -		log_info("Scanning disk %s...\n", dev->name);
> -		ret = efi_disk_add_dev(NULL, NULL, if_typename,
> -					desc, desc->devnum, NULL, 0, &disk);
> -		if (ret == EFI_NOT_READY) {
> -			log_notice("Disk %s not ready\n", dev->name);
> -			continue;
> -		}
> -		if (ret) {
> -			log_err("ERROR: failure to add disk device %s, r = %lu\n",
> -				dev->name, ret & ~EFI_ERROR_MASK);
> -			continue;
> -		}
> -		disks++;
> +	/* TODO: We won't support partitions in a partition */
> +	if (id != UCLASS_BLK) {
> +		if (id != UCLASS_PARTITION)
> +			log_info("Not a block device: %s\n", dev->name);

I get the following messages on the sandbox:

Not a block device: pinmux_i2c0_pins
Not a block device: i2c@0
Not a block device: rtc@61
Not a block device: bootcount@0
Not a block device: emul
Not a block device: emull

Why do you create these?

Best regards

Heinrich

> +		return 0;
> +	}
> +
> +	ret = efi_disk_create_raw(dev);
> +	if (ret)
> +		return -1;
>
> -		/* Partitions show up as block devices in EFI */
> -		disks += efi_disk_create_partitions(
> -					&disk->header, desc, if_typename,
> -					desc->devnum, dev->name);
> +	device_foreach_child(child, dev) {
> +		ret = efi_disk_create_part(child);
> +		if (ret)
> +			return -1;
>   	}
>
> -	log_info("Found %d disks\n", disks);
> +	return 0;
> +}
> +
> +efi_status_t efi_disk_init(void)
> +{
> +	int ret;
> +
> +	ret = event_register("efi_disk add", EVT_DM_POST_PROBE,
> +			     efi_disk_probe, NULL);
> +	if (ret) {
> +		log_err("Event registration for efi_disk add failed\n");
> +		return EFI_OUT_OF_RESOURCES;
> +	}
>
>   	return EFI_SUCCESS;
>   }
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 1aba71cd9624..1c912b0157aa 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -200,11 +200,12 @@ efi_status_t efi_init_obj_list(void)
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>
> -#ifdef CONFIG_PARTITIONS
> -	ret = efi_disk_register();
> -	if (ret != EFI_SUCCESS)
> -		goto out;
> -#endif
> +	if (IS_ENABLED(CONFIG_BLK)) {
> +		ret = efi_disk_init();
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
>   	if (IS_ENABLED(CONFIG_EFI_RNG_PROTOCOL)) {
>   		ret = efi_rng_register();
>   		if (ret != EFI_SUCCESS)

Re: [RFC v2 19/20] efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI) devices

[Heinrich Schuchardt]

On 12/10/21 07:49, AKASHI Takahiro wrote:
> When we create an efi_disk device with an UEFI application using driver
> binding protocol, the 'efi_driver' framework tries to create
> a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to

I assume this is IF_TYPE_EFI_LOADER now?

Best regards

Heinrich

> calling a PROBE callback, efi_disk_probe().
> In this case, however, we don't need to create another "efi_disk" device
> as we already have this device instance.
>
> So we should avoid recursively invoke further processing in the callback
> function.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   lib/efi_loader/efi_disk.c | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 8e33af76711f..f37f8c1ab0f1 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -603,6 +603,7 @@ static int efi_disk_probe(void *ctx, struct event *event)
>   {
>   	struct udevice *dev;
>   	enum uclass_id id;
> +	struct blk_desc *desc;
>   	struct udevice *child;
>   	int ret;
>
> @@ -616,9 +617,16 @@ static int efi_disk_probe(void *ctx, struct event *event)
>   		return 0;
>   	}
>
> -	ret = efi_disk_create_raw(dev);
> -	if (ret)
> -		return -1;
> +	/*
> +	 * avoid creating duplicated objects now that efi_driver
> +	 * has already created an efi_disk at this moment.
> +	 */
> +	desc = dev_get_uclass_plat(dev);
> +	if (desc->if_type != IF_TYPE_EFI) {
> +		ret = efi_disk_create_raw(dev);
> +		if (ret)
> +			return -1;
> +	}
>
>   	device_foreach_child(child, dev) {
>   		ret = efi_disk_create_part(child);
> @@ -642,13 +650,17 @@ static int efi_disk_probe(void *ctx, struct event *event)
>   static int efi_disk_delete_raw(struct udevice *dev)
>   {
>   	efi_handle_t handle;
> +	struct blk_desc *desc;
>   	struct efi_disk_obj *diskobj;
>
>   	if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
>   		return -1;
>
> -	diskobj = container_of(handle, struct efi_disk_obj, header);
> -	efi_free_pool(diskobj->dp);
> +	desc = dev_get_uclass_plat(dev);
> +	if (desc->if_type != IF_TYPE_EFI) {
> +		diskobj = container_of(handle, struct efi_disk_obj, header);
> +		efi_free_pool(diskobj->dp);
> +	}
>
>   	efi_delete_handle(handle);
>   	dev_tag_del(dev, DM_TAG_EFI);

Re: [RFC v2 11/20] dm: add event notification

[Heinrich Schuchardt]

On 12/10/21 07:49, AKASHI Takahiro wrote:
> From: Simon Glass <sjg@chromium.org>
>
> This is a draft implementation of event notification mechanism from Simon.
> Under this scheme, any U-Boot subsystem can register some kind of callback
> function to a particular event (more event types will be added later) and
> that function will be invoked once the event is fired.
>
> As a first user, UEFI subsystem makes use of PROBE and REMOVE events so
> that we will be able to automatically create/remove efi_disk objects
> relying on associated block devices (UCLASS_BLK).
>
> To run the test:
> ./u-boot -T -c "ut common test_event_probe"
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> [add REMOVE event; fix checkpatch warnings]
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

This patch breaks building sandbox_spl_defconfig.

common/event.c: In function ‘event_register’:
common/event.c:29:39: error: ‘gd_t’ {aka ‘struct global_data’} has no
member named ‘event_state’
    29 |         struct event_state *state = gd->event_state;
       |                                       ^~
common/event.c: In function ‘event_notify’:
common/event.c:47:39: error: ‘gd_t’ {aka ‘struct global_data’} has no
member named ‘event_state’
    47 |         struct event_state *state = gd->event_state;
       |                                       ^~
common/event.c: At top level:
common/event.c:77:5: error: redefinition of ‘event_uninit’
    77 | int event_uninit(void)
       |     ^~~~~~~~~~~~
In file included from common/event.c:13:
include/event.h:94:19: note: previous definition of ‘event_uninit’ with
type ‘int(void)’
    94 | static inline int event_uninit(void)
       |                   ^~~~~~~~~~~~
common/event.c: In function ‘event_uninit’:
common/event.c:79:39: error: ‘gd_t’ {aka ‘struct global_data’} has no
member named ‘event_state’
    79 |         struct event_state *state = gd->event_state;
       |                                       ^~
common/event.c: At top level:
common/event.c:90:5: error: redefinition of ‘event_init’
    90 | int event_init(void)
       |     ^~~~~~~~~~
In file included from common/event.c:13:
include/event.h:99:19: note: previous definition of ‘event_init’ with
type ‘int(void)’
    99 | static inline int event_init(void)
       |                   ^~~~~~~~~~
common/event.c: In function ‘event_init’:
common/event.c:100:11: error: ‘gd_t’ {aka ‘struct global_data’} has no
member named ‘event_state’
   100 |         gd->event_state = state;
       |           ^~
make[2]: *** [scripts/Makefile.build:254: spl/common/event.o] Error 1
make[1]: *** [scripts/Makefile.spl:515: spl/common] Error 2
make[1]: *** Waiting for unfinished jobs....
   AR      spl/env/built-in.o
   AR      spl/common/spl/built-in.o
test/test-main.c: In function ‘test_pre_run’:
test/test-main.c:222:11: error: ‘gd_t’ {aka ‘struct global_data’} has no
member named ‘event_state’
   222 |         gd->event_state = NULL;
       |           ^~
make[2]: *** [scripts/Makefile.build:253: spl/test/test-main.o] Error 1
make[2]: *** Waiting for unfinished jobs....
   AR      spl/test/image/built-in.o
make[1]: *** [scripts/Makefile.spl:515: spl/test] Error 2
make: *** [Makefile:2086: spl/u-boot-spl] Error 2

Best regards

Heinrich

Re: [RFC v2 05/20] mmc: call device_probe() after scanning

[Heinrich Schuchardt]

On 12/10/21 07:49, AKASHI Takahiro wrote:
> Every time a mmc bus/port is scanned and a new device is detected,
> we want to call device_probe() as it will give us a chance to run
> additional post-processings for some purposes.
>
> In particular, support for creating partitions on a device will be added.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   drivers/mmc/mmc-uclass.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 3ee92d03ca23..6c907b65fde7 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -418,6 +418,7 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
>   	bdesc->part_type = cfg->part_type;
>   	mmc->dev = dev;
>   	mmc->user_speed_mode = MMC_MODES_END;
> +
>   	return 0;
>   }
>
> @@ -467,6 +468,18 @@ static int mmc_blk_probe(struct udevice *dev)
>   		return ret;
>   	}
>
> +	ret = device_probe(dev);
> +	if (ret) {
> +		debug("Can't probe\n");
> +
> +		if (IS_ENABLED(CONFIG_MMC_UHS_SUPPORT) ||
> +		    IS_ENABLED(CONFIG_MMC_HS200_SUPPORT) ||
> +		    IS_ENABLED(CONFIG_MMC_HS400_SUPPORT))
> +			mmc_deinit(mmc);

This results in a warning when compiling sandbox_spl_defconfig:

drivers/mmc/mmc-uclass.c: In function ‘mmc_blk_probe’:
drivers/mmc/mmc-uclass.c:478:25: warning: implicit declaration of
function ‘mmc_deinit’; did you mean ‘mmc_reinit’?
[-Wimplicit-function-declaration]
   478 |                         mmc_deinit(mmc);
       |                         ^~~~~~~~~~
       |                         mmc_reinit

Best regards

Heinrich

> +
> +		return ret;
> +	}
> +
>   	return 0;
>   }
>

Re: [RFC v2 05/20] mmc: call device_probe() after scanning

[Jaehoon Chung]

Dear AKASHI,

On 12/10/21 3:49 PM, AKASHI Takahiro wrote:
> Every time a mmc bus/port is scanned and a new device is detected,
> we want to call device_probe() as it will give us a chance to run
> additional post-processings for some purposes.
> 
> In particular, support for creating partitions on a device will be added.

If add me and Peng as Cc when you sent the patches related with mmc, it's more helpful to review.

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  drivers/mmc/mmc-uclass.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 3ee92d03ca23..6c907b65fde7 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -418,6 +418,7 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
>  	bdesc->part_type = cfg->part_type;
>  	mmc->dev = dev;
>  	mmc->user_speed_mode = MMC_MODES_END;
> +
>  	return 0;
>  }
>  
> @@ -467,6 +468,18 @@ static int mmc_blk_probe(struct udevice *dev)
>  		return ret;
>  	}
>  
> +	ret = device_probe(dev);
> +	if (ret) {
> +		debug("Can't probe\n");
> +
> +		if (IS_ENABLED(CONFIG_MMC_UHS_SUPPORT) ||
> +		    IS_ENABLED(CONFIG_MMC_HS200_SUPPORT) ||
> +		    IS_ENABLED(CONFIG_MMC_HS400_SUPPORT))
> +			mmc_deinit(mmc);
> +
> +		return ret;
> +	}
> +
>  	return 0;
>  }
>  
> 

Re: [RFC v2 19/20] efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI) devices

[AKASHI Takahiro]

On Sun, Jan 02, 2022 at 10:19:21AM +0100, Heinrich Schuchardt wrote:
> On 12/10/21 07:49, AKASHI Takahiro wrote:
> > When we create an efi_disk device with an UEFI application using driver
> > binding protocol, the 'efi_driver' framework tries to create
> > a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to
> 
> I assume this is IF_TYPE_EFI_LOADER now?

Yes in v2022.01-rc4 and later.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> > calling a PROBE callback, efi_disk_probe().
> > In this case, however, we don't need to create another "efi_disk" device
> > as we already have this device instance.
> > 
> > So we should avoid recursively invoke further processing in the callback
> > function.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   lib/efi_loader/efi_disk.c | 22 +++++++++++++++++-----
> >   1 file changed, 17 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index 8e33af76711f..f37f8c1ab0f1 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -603,6 +603,7 @@ static int efi_disk_probe(void *ctx, struct event *event)
> >   {
> >   	struct udevice *dev;
> >   	enum uclass_id id;
> > +	struct blk_desc *desc;
> >   	struct udevice *child;
> >   	int ret;
> > 
> > @@ -616,9 +617,16 @@ static int efi_disk_probe(void *ctx, struct event *event)
> >   		return 0;
> >   	}
> > 
> > -	ret = efi_disk_create_raw(dev);
> > -	if (ret)
> > -		return -1;
> > +	/*
> > +	 * avoid creating duplicated objects now that efi_driver
> > +	 * has already created an efi_disk at this moment.
> > +	 */
> > +	desc = dev_get_uclass_plat(dev);
> > +	if (desc->if_type != IF_TYPE_EFI) {
> > +		ret = efi_disk_create_raw(dev);
> > +		if (ret)
> > +			return -1;
> > +	}
> > 
> >   	device_foreach_child(child, dev) {
> >   		ret = efi_disk_create_part(child);
> > @@ -642,13 +650,17 @@ static int efi_disk_probe(void *ctx, struct event *event)
> >   static int efi_disk_delete_raw(struct udevice *dev)
> >   {
> >   	efi_handle_t handle;
> > +	struct blk_desc *desc;
> >   	struct efi_disk_obj *diskobj;
> > 
> >   	if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
> >   		return -1;
> > 
> > -	diskobj = container_of(handle, struct efi_disk_obj, header);
> > -	efi_free_pool(diskobj->dp);
> > +	desc = dev_get_uclass_plat(dev);
> > +	if (desc->if_type != IF_TYPE_EFI) {
> > +		diskobj = container_of(handle, struct efi_disk_obj, header);
> > +		efi_free_pool(diskobj->dp);
> > +	}
> > 
> >   	efi_delete_handle(handle);
> >   	dev_tag_del(dev, DM_TAG_EFI);
> 

Re: [RFC v2 15/20] efi_loader: disk: a helper function to create efi_disk objects from udevice

[AKASHI Takahiro]

On Sun, Jan 02, 2022 at 10:18:18AM +0100, Heinrich Schuchardt wrote:
> On 12/10/21 07:49, AKASHI Takahiro wrote:
> > Add efi_disk_probe() function.
> > This function creates an efi_disk object for a raw disk device (UCLASS_BLK)
> > and additional objects for related partitions (UCLASS_PARTITION).
> > 
> > So this function is expected to be called through driver model's "probe"
> > interface every time one raw disk device is detected and activated.
> > We assume that partition devices (UCLASS_PARTITION) have been created
> > when this function is invoked.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   include/efi_loader.h       |   4 +-
> >   lib/efi_loader/Kconfig     |   2 +
> >   lib/efi_loader/efi_disk.c  | 206 ++++++++++++++++++++++++-------------
> >   lib/efi_loader/efi_setup.c |  11 +-
> >   4 files changed, 142 insertions(+), 81 deletions(-)
> > 
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index d52e399841ba..a51095930efa 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -519,8 +519,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition,
> >   void efi_carve_out_dt_rsv(void *fdt);
> >   /* Called by bootefi to make console interface available */
> >   efi_status_t efi_console_register(void);
> > -/* Called by bootefi to make all disk storage accessible as EFI objects */
> > -efi_status_t efi_disk_register(void);
> > +/* Called by efi_init_obj_list() to initialize efi_disks */
> > +efi_status_t efi_disk_init(void);
> >   /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
> >   efi_status_t efi_rng_register(void);
> >   /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 700dc838ddb9..108c00343fce 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -11,6 +11,7 @@ config EFI_LOADER
> >   	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
> >   	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
> >   	depends on BLK
> > +	depends on EVENT
> >   	depends on DM_ETH || !NET
> >   	depends on !EFI_APP
> >   	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
> > @@ -41,6 +42,7 @@ config CMD_BOOTEFI_BOOTMGR
> > 
> >   config EFI_SETUP_EARLY
> >   	bool
> > +	default y
> > 
> >   choice
> >   	prompt "Store for non-volatile UEFI variables"
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index 45127d176869..2941b0c3db47 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -10,6 +10,9 @@
> >   #include <common.h>
> >   #include <blk.h>
> >   #include <dm.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/tag.h>
> > +#include <event.h>
> >   #include <efi_loader.h>
> >   #include <fs.h>
> >   #include <log.h>
> > @@ -487,103 +490,158 @@ error:
> >   	return ret;
> >   }
> > 
> > -/**
> > - * efi_disk_create_partitions() - create handles and protocols for partitions
> > +/*
> > + * Create a handle for a whole raw disk
> >    *
> > - * Create handles and protocols for the partitions of a block device.
> > + * @dev		uclass device (UCLASS_BLK)
> >    *
> > - * @parent:		handle of the parent disk
> > - * @desc:		block device
> > - * @if_typename:	interface type
> > - * @diskid:		device number
> > - * @pdevname:		device name
> > - * Return:		number of partitions created
> > + * Create an efi_disk object which is associated with @dev.
> > + * The type of @dev must be UCLASS_BLK.
> > + *
> > + * @return	0 on success, -1 otherwise
> >    */
> > -int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
> > -			       const char *if_typename, int diskid,
> > -			       const char *pdevname)
> > +static int efi_disk_create_raw(struct udevice *dev)
> >   {
> > -	int disks = 0;
> > -	char devname[32] = { 0 }; /* dp->str is u16[32] long */
> > -	int part;
> > -	struct efi_device_path *dp = NULL;
> > +	struct efi_disk_obj *disk;
> > +	struct blk_desc *desc;
> > +	const char *if_typename;
> > +	int diskid;
> >   	efi_status_t ret;
> > -	struct efi_handler *handler;
> > 
> > -	/* Get the device path of the parent */
> > -	ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
> > -	if (ret == EFI_SUCCESS)
> > -		dp = handler->protocol_interface;
> > -
> > -	/* Add devices for each partition */
> > -	for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
> > -		struct disk_partition info;
> > -
> > -		if (part_get_info(desc, part, &info))
> > -			continue;
> > -		snprintf(devname, sizeof(devname), "%s:%x", pdevname,
> > -			 part);
> > -		ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
> > -				       &info, part, NULL);
> > -		if (ret != EFI_SUCCESS) {
> > -			log_err("Adding partition %s failed\n", pdevname);
> > -			continue;
> > -		}
> > -		disks++;
> > +	desc = dev_get_uclass_plat(dev);
> > +	if_typename = blk_get_if_type_name(desc->if_type);
> > +	diskid = desc->devnum;
> > +
> > +	ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
> > +			       diskid, NULL, 0, &disk);
> > +	if (ret != EFI_SUCCESS) {
> > +		if (ret == EFI_NOT_READY)
> > +			log_notice("Disk %s not ready\n", dev->name);
> > +		else
> > +			log_err("Adding disk for %s failed\n", dev->name);
> > +
> > +		return -1;
> > +	}
> > +	disk->dev = dev;
> > +	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
> > +		efi_free_pool(disk->dp);
> > +		efi_delete_handle(&disk->header);
> > +
> > +		return -1;
> >   	}
> > 
> > -	return disks;
> > +	return 0;
> >   }
> > 
> > -/**
> > - * efi_disk_register() - register block devices
> > - *
> > - * U-Boot doesn't have a list of all online disk devices. So when running our
> > - * EFI payload, we scan through all of the potentially available ones and
> > - * store them in our object pool.
> > +/*
> > + * Create a handle for a disk partition
> >    *
> > - * This function is called in efi_init_obj_list().
> > + * @dev		uclass device (UCLASS_PARTITION)
> >    *
> > - * TODO(sjg@chromium.org): Actually with CONFIG_BLK, U-Boot does have this.
> > - * Consider converting the code to look up devices as needed. The EFI device
> > - * could be a child of the UCLASS_BLK block device, perhaps.
> > + * Create an efi_disk object which is associated with @dev.
> > + * The type of @dev must be UCLASS_PARTITION.
> >    *
> > - * Return:	status code
> > + * @return	0 on success, -1 otherwise
> >    */
> > -efi_status_t efi_disk_register(void)
> > +static int efi_disk_create_part(struct udevice *dev)
> >   {
> > +	efi_handle_t parent;
> > +	struct blk_desc *desc;
> > +	const char *if_typename;
> > +	struct disk_part *part_data;
> > +	struct disk_partition *info;
> > +	unsigned int part;
> > +	int diskid;
> > +	struct efi_handler *handler;
> > +	struct efi_device_path *dp_parent;
> >   	struct efi_disk_obj *disk;
> > -	int disks = 0;
> >   	efi_status_t ret;
> > +
> > +	if (dev_tag_get_ptr(dev_get_parent(dev), DM_TAG_EFI, (void **)&parent))
> > +		return -1;
> > +
> > +	desc = dev_get_uclass_plat(dev_get_parent(dev));
> > +	if_typename = blk_get_if_type_name(desc->if_type);
> > +	diskid = desc->devnum;
> > +
> > +	part_data = dev_get_uclass_plat(dev);
> > +	part = part_data->partnum;
> > +	info = &part_data->gpt_part_info;
> > +
> > +	ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
> > +	if (ret != EFI_SUCCESS)
> > +		return -1;
> > +	dp_parent = (struct efi_device_path *)handler->protocol_interface;
> > +
> > +	ret = efi_disk_add_dev(parent, dp_parent, if_typename, desc, diskid,
> > +			       info, part, &disk);
> > +	if (ret != EFI_SUCCESS) {
> > +		log_err("Adding partition for %s failed\n", dev->name);
> > +		return -1;
> > +	}
> > +	disk->dev = dev;
> > +	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
> > +		efi_free_pool(disk->dp);
> > +		efi_delete_handle(&disk->header);
> > +
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * Create efi_disk objects for a block device
> > + *
> > + * @dev		uclass device (UCLASS_BLK)
> > + *
> > + * Create efi_disk objects for partitions as well as a raw disk
> > + * which is associated with @dev.
> > + * The type of @dev must be UCLASS_BLK.
> > + * This function is expected to be called at EV_PM_POST_PROBE.
> > + *
> > + * @return	0 on success, -1 otherwise
> > + */
> > +static int efi_disk_probe(void *ctx, struct event *event)
> > +{
> >   	struct udevice *dev;
> > +	enum uclass_id id;
> > +	struct udevice *child;
> > +	int ret;
> > 
> > -	for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
> > -	     uclass_next_device_check(&dev)) {
> > -		struct blk_desc *desc = dev_get_uclass_plat(dev);
> > -		const char *if_typename = blk_get_if_type_name(desc->if_type);
> > +	dev = event->data.dm.dev;
> > +	id = device_get_uclass_id(dev);
> > 
> > -		/* Add block device for the full device */
> > -		log_info("Scanning disk %s...\n", dev->name);
> > -		ret = efi_disk_add_dev(NULL, NULL, if_typename,
> > -					desc, desc->devnum, NULL, 0, &disk);
> > -		if (ret == EFI_NOT_READY) {
> > -			log_notice("Disk %s not ready\n", dev->name);
> > -			continue;
> > -		}
> > -		if (ret) {
> > -			log_err("ERROR: failure to add disk device %s, r = %lu\n",
> > -				dev->name, ret & ~EFI_ERROR_MASK);
> > -			continue;
> > -		}
> > -		disks++;
> > +	/* TODO: We won't support partitions in a partition */
> > +	if (id != UCLASS_BLK) {
> > +		if (id != UCLASS_PARTITION)
> > +			log_info("Not a block device: %s\n", dev->name);
> 
> I get the following messages on the sandbox:
> 
> Not a block device: pinmux_i2c0_pins
> Not a block device: i2c@0
> Not a block device: rtc@61
> Not a block device: bootcount@0
> Not a block device: emul
> Not a block device: emull
> 
> Why do you create these?

They are debug messages as I intended to make sure that a 'probe' hook
was always called in vain even if a given udevice was not a block device.
(In this sense, event API has room for improvement.)

On the other hand, this behavior is useful when we want to add a nested
partition, that is, partitions in a partition.

Will drop the message for now.

-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> > +		return 0;
> > +	}
> > +
> > +	ret = efi_disk_create_raw(dev);
> > +	if (ret)
> > +		return -1;
> > 
> > -		/* Partitions show up as block devices in EFI */
> > -		disks += efi_disk_create_partitions(
> > -					&disk->header, desc, if_typename,
> > -					desc->devnum, dev->name);
> > +	device_foreach_child(child, dev) {
> > +		ret = efi_disk_create_part(child);
> > +		if (ret)
> > +			return -1;
> >   	}
> > 
> > -	log_info("Found %d disks\n", disks);
> > +	return 0;
> > +}
> > +
> > +efi_status_t efi_disk_init(void)
> > +{
> > +	int ret;
> > +
> > +	ret = event_register("efi_disk add", EVT_DM_POST_PROBE,
> > +			     efi_disk_probe, NULL);
> > +	if (ret) {
> > +		log_err("Event registration for efi_disk add failed\n");
> > +		return EFI_OUT_OF_RESOURCES;
> > +	}
> > 
> >   	return EFI_SUCCESS;
> >   }
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 1aba71cd9624..1c912b0157aa 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -200,11 +200,12 @@ efi_status_t efi_init_obj_list(void)
> >   	if (ret != EFI_SUCCESS)
> >   		goto out;
> > 
> > -#ifdef CONFIG_PARTITIONS
> > -	ret = efi_disk_register();
> > -	if (ret != EFI_SUCCESS)
> > -		goto out;
> > -#endif
> > +	if (IS_ENABLED(CONFIG_BLK)) {
> > +		ret = efi_disk_init();
> > +		if (ret != EFI_SUCCESS)
> > +			goto out;
> > +	}
> > +
> >   	if (IS_ENABLED(CONFIG_EFI_RNG_PROTOCOL)) {
> >   		ret = efi_rng_register();
> >   		if (ret != EFI_SUCCESS)
> 

Re: [RFC v2 09/20] dm: fix an 'undefined' error in some macros

[Ilias Apalodimas]

On Mon, 13 Dec 2021 at 14:51, Simon Glass <sjg@chromium.org> wrote:
>
> On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >
> > Due to a non-existing parameter name in macro's, use of those macro's will
> > cause a compiler error of "undefined reference".
> > Unfortunately, dm test doesn't fail because a wrong name ("&dev", hence it
> > is accidentally a valid name in the context of a caller site) is passed on.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Fixes: f262d4ca4b2b ("dm: core: Add a way to read platdata for all
> >         child devices")
> > Fixes: 903e83ee8464 ("dm: core: Add a way to iterate through children,
> >         probing each")
> > ---
> >  include/dm/device.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [RFC v2 00/20] efi_loader: more tightly integrate UEFI disks to driver model

[Simon Glass]

Hi Takahiro,

On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> # This is a kind of snapshot of my current work.
> # I submit this to show my progress on this effort, and then
> # I haven't addressed all the issues in this version.
> # See the change history below.
>
> The purpose of this RPC is to reignite the discussion about how UEFI
> subystem would best be integrated into U-Boot driver model.
> In the past, I poposed a couple of patch series, the latest one[1],
> while Heinrich revealed his idea[2], and the approach taken here is
> something between them, with a focus on block device handlings.

Any update on this series please?

Regards,
Simon

Re: [RFC v2 00/20] efi_loader: more tightly integrate UEFI disks to driver model

[AKASHI Takahiro]

Hi, Simon,


On Thu, Jan 27, 2022 at 08:05:56AM -0700, Simon Glass wrote:
> Hi Takahiro,
> 
> On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >
> > # This is a kind of snapshot of my current work.
> > # I submit this to show my progress on this effort, and then
> > # I haven't addressed all the issues in this version.
> > # See the change history below.
> >
> > The purpose of this RPC is to reignite the discussion about how UEFI
> > subystem would best be integrated into U-Boot driver model.
> > In the past, I poposed a couple of patch series, the latest one[1],
> > while Heinrich revealed his idea[2], and the approach taken here is
> > something between them, with a focus on block device handlings.
> 
> Any update on this series please?

Thank you for your concern.
While I have made no progress so far, most of comments I've got
look trivial. I gonna have to double-check, though.

So do you have any update on your part of event notification (patch11/20)?

-Takahiro Akashi

> Regards,
> Simon

Re: [RFC v2 00/20] efi_loader: more tightly integrate UEFI disks to driver model

[AKASHI Takahiro]

On Fri, Jan 28, 2022 at 02:01:41PM +0900, AKASHI Takahiro wrote:
> Hi, Simon,
> 
> 
> On Thu, Jan 27, 2022 at 08:05:56AM -0700, Simon Glass wrote:
> > Hi Takahiro,
> > 
> > On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > >
> > > # This is a kind of snapshot of my current work.
> > > # I submit this to show my progress on this effort, and then
> > > # I haven't addressed all the issues in this version.
> > > # See the change history below.
> > >
> > > The purpose of this RPC is to reignite the discussion about how UEFI
> > > subystem would best be integrated into U-Boot driver model.
> > > In the past, I poposed a couple of patch series, the latest one[1],
> > > while Heinrich revealed his idea[2], and the approach taken here is
> > > something between them, with a focus on block device handlings.
> > 
> > Any update on this series please?
> 
> Thank you for your concern.
> While I have made no progress so far, most of comments I've got
> look trivial. I gonna have to double-check, though.
> 
> So do you have any update on your part of event notification (patch11/20)?

I checked all the comments.
If you have no update, I will be able to submit a new version
early next week.

-Takahiro Akashi

> -Takahiro Akashi
> 
> > Regards,
> > Simon