[PATCH v5 13/15] dm: core: Switch uclass_*_device_err to use uclass_*_device_check

Michal Suchanek msuchanek at suse.de
Tue Sep 27 23:38:05 CEST 2022


The _err variant iterators use the simple iterators without suffix as
basis.

However, there is no user that uclass_next_device_err for iteration,
many users of uclass_first_device_err use it to get the first and
(assumed) only device of an uclass, and a couple that use
uclass_next_device_err to get the device following a known device in the
uclass list.

While there are some truly singleton device classes in which more than
one device cannot exist these are quite rare, and most classes can have
multiple devices even if it is not the case on the SoC's EVB.

In a later patch the simple iterators will be updated to not stop on
error and return next device instead. With this in many cases the code
that expects the first device or an error if it fails to probe may get
the next device instead. Use the _check iterators as the basis of _err
iterators to preserve the old behavior.

This is problematic for eth_get_dev: it relies on the broken behavior
that returns an error but not the device on which the error happened
which gives the caller no reasonable way to report or handle the error.
With this change the device is returned but eth_get_dev stores the
returned device pointer directly in a global state without checking the
return value. Unset the pointer again in the error case.

Do the same for sysinfo_get because it is not clear how exactly the
sysinfo is used by some callers, and fix up a call to pci_get_bus that
checks the returned device and not the return value.

Switch uclass_foreach_dev_probe to the simple iterator - it does not
use the returned value, and did not give out the failing devices
previously.

Note in documentation that a non-activated device can be returned on
error.

Signed-off-by: Michal Suchanek <msuchanek at suse.de>
---
v5: - udate documentation
    - fix up a few more cases where device returned on error may cause
      problem
---
 drivers/core/uclass.c            | 28 ++++++++++++-------------
 drivers/pci/pci-uclass.c         |  7 +++----
 drivers/sysinfo/sysinfo-uclass.c | 10 ++++++++-
 include/dm/uclass.h              | 36 ++++++++++++++++++--------------
 net/eth-uclass.c                 |  2 ++
 5 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index a591e22403..b7d11bdd23 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -586,19 +586,6 @@ int uclass_first_device(enum uclass_id id, struct udevice **devp)
 	return uclass_get_device_tail(dev, ret, devp);
 }
 
-int uclass_first_device_err(enum uclass_id id, struct udevice **devp)
-{
-	int ret;
-
-	ret = uclass_first_device(id, devp);
-	if (ret)
-		return ret;
-	else if (!*devp)
-		return -ENODEV;
-
-	return 0;
-}
-
 int uclass_next_device(struct udevice **devp)
 {
 	struct udevice *dev = *devp;
@@ -611,11 +598,24 @@ int uclass_next_device(struct udevice **devp)
 	return uclass_get_device_tail(dev, ret, devp);
 }
 
+int uclass_first_device_err(enum uclass_id id, struct udevice **devp)
+{
+	int ret;
+
+	ret = uclass_first_device_check(id, devp);
+	if (ret)
+		return ret;
+	else if (!*devp)
+		return -ENODEV;
+
+	return 0;
+}
+
 int uclass_next_device_err(struct udevice **devp)
 {
 	int ret;
 
-	ret = uclass_next_device(devp);
+	ret = uclass_next_device_check(devp);
 	if (ret)
 		return ret;
 	else if (!*devp)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 00e3828d95..2aa1043604 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -1768,10 +1768,9 @@ int pci_sriov_init(struct udevice *pdev, int vf_en)
 
 	bdf = dm_pci_get_bdf(pdev);
 
-	pci_get_bus(PCI_BUS(bdf), &bus);
-
-	if (!bus)
-		return -ENODEV;
+	ret = pci_get_bus(PCI_BUS(bdf), &bus);
+	if (ret)
+		return ret;
 
 	bdf += PCI_BDF(0, 0, vf_offset);
 
diff --git a/drivers/sysinfo/sysinfo-uclass.c b/drivers/sysinfo/sysinfo-uclass.c
index c5cc3cb959..10194d0e14 100644
--- a/drivers/sysinfo/sysinfo-uclass.c
+++ b/drivers/sysinfo/sysinfo-uclass.c
@@ -16,7 +16,15 @@ struct sysinfo_priv {
 
 int sysinfo_get(struct udevice **devp)
 {
-	return uclass_first_device_err(UCLASS_SYSINFO, devp);
+	int ret = uclass_first_device_err(UCLASS_SYSINFO, devp);
+
+	/*
+	 * There is some very dodgy error handling in gazerbeam,
+	 * do not return a device on error.
+	 */
+	if (ret)
+		*devp = NULL;
+	return ret;
 }
 
 int sysinfo_detect(struct udevice *dev)
diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index f6c0110b06..b1c016ef9f 100644
--- a/include/dm/uclass.h
+++ b/include/dm/uclass.h
@@ -332,17 +332,6 @@ int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv,
  */
 int uclass_first_device(enum uclass_id id, struct udevice **devp);
 
-/**
- * uclass_first_device_err() - Get the first device in a uclass
- *
- * The device returned is probed if necessary, and ready for use
- *
- * @id: Uclass ID to look up
- * @devp: Returns pointer to the first device in that uclass, or NULL if none
- * Return: 0 if found, -ENODEV if not found, other -ve on error
- */
-int uclass_first_device_err(enum uclass_id id, struct udevice **devp);
-
 /**
  * uclass_next_device() - Get the next device in a uclass
  *
@@ -358,10 +347,23 @@ int uclass_first_device_err(enum uclass_id id, struct udevice **devp);
  */
 int uclass_next_device(struct udevice **devp);
 
+/**
+ * uclass_first_device_err() - Get the first device in a uclass
+ *
+ * The device returned is probed if necessary, and ready for use if no error is
+ * returned
+ *
+ * @id: Uclass ID to look up
+ * @devp: Returns pointer to the first device in that uclass, or NULL if none
+ * Return: 0 if found, -ENODEV if not found, other -ve on error
+ */
+int uclass_first_device_err(enum uclass_id id, struct udevice **devp);
+
 /**
  * uclass_next_device_err() - Get the next device in a uclass
  *
- * The device returned is probed if necessary, and ready for use
+ * The device returned is probed if necessary, and ready for use if no error is
+ * returned
  *
  * @devp: On entry, pointer to device to lookup. On exit, returns pointer
  * to the next device in the uclass if no error occurred, or NULL if
@@ -373,7 +375,8 @@ int uclass_next_device_err(struct udevice **devp);
 /**
  * uclass_first_device_check() - Get the first device in a uclass
  *
- * The device returned is probed if necessary, and ready for use
+ * The device returned is probed if necessary, and ready for use if no error is
+ * returned
  *
  * This function is useful to start iterating through a list of devices which
  * are functioning correctly and can be probed.
@@ -389,7 +392,8 @@ int uclass_first_device_check(enum uclass_id id, struct udevice **devp);
 /**
  * uclass_next_device_check() - Get the next device in a uclass
  *
- * The device returned is probed if necessary, and ready for use
+ * The device returned is probed if necessary, and ready for use if no error is
+ * returned
  *
  * This function is useful to start iterating through a list of devices which
  * are functioning correctly and can be probed.
@@ -491,7 +495,7 @@ int uclass_id_count(enum uclass_id id);
  * are no more devices.
  */
 #define uclass_foreach_dev_probe(id, dev)	\
-	for (int _ret = uclass_first_device_err(id, &dev); !_ret && dev; \
-	     _ret = uclass_next_device_err(&dev))
+	for (uclass_first_device(id, &dev); dev; \
+	     uclass_next_device(&dev))
 
 #endif
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 8c3f9cc31b..f41da4b37b 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -93,6 +93,8 @@ struct udevice *eth_get_dev(void)
 		if (eth_errno)
 			eth_errno = uclass_first_device_err(UCLASS_ETH,
 							    &uc_priv->current);
+		if (eth_errno)
+			uc_priv->current = NULL;
 	}
 	return uc_priv->current;
 }
-- 
2.37.3



More information about the U-Boot mailing list