[U-Boot] [PATCH v2 1/3] dm: core: Clarify uclass_first/next_device() comments

[U-Boot] [PATCH v2 1/3] dm: core: Clarify uclass_first/next_device() comments

[Simon Glass]

These are not as clear as they could be. Tidy them up a bit. Also fix a
tiny code-style nit.

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

Changes in v2: None

 drivers/core/uclass.c |  3 +--
 include/dm/uclass.h   | 13 +++++++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index 04fb45b01a..42613031ff 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -342,8 +342,7 @@ int uclass_get_device_by_driver(enum uclass_id id,
 	return -ENODEV;
 }
 
-int uclass_get_device_tail(struct udevice *dev, int ret,
-				  struct udevice **devp)
+int uclass_get_device_tail(struct udevice *dev, int ret, struct udevice **devp)
 {
 	if (ret)
 		return ret;
diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index b583aa869b..63da868ae5 100644
--- a/include/dm/uclass.h
+++ b/include/dm/uclass.h
@@ -224,8 +224,13 @@ int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv,
  *
  * The device returned is probed if necessary, and ready for use
  *
+ * This function is useful to start iterating through a list of devices which
+ * are functioning correctly and can be probed.
+ *
  * @id: Uclass ID to look up
- * @devp: Returns pointer to the first device in that uclass, or NULL if none
+ * @devp: Returns pointer to the first device in that uclass if no error
+ * occurred, or NULL if there is no first device, or an error occurred with
+ * that device.
  * @return 0 if OK (found or not found), other -ve on error
  */
 int uclass_first_device(enum uclass_id id, struct udevice **devp);
@@ -246,8 +251,12 @@ int uclass_first_device_err(enum uclass_id id, struct udevice **devp);
  *
  * The device returned is probed if necessary, and ready for use
  *
+ * This function is useful to start iterating through a list of devices which
+ * are functioning correctly and can be probed.
+ *
  * @devp: On entry, pointer to device to lookup. On exit, returns pointer
- * to the next device in the same uclass, or NULL if none
+ * to the next device in the uclass if no error occurred, or NULL if there is
+ * no next device, or an error occurred with that next device.
  * @return 0 if OK (found or not found), other -ve on error
  */
 int uclass_next_device(struct udevice **devp);
-- 
2.12.2.816.g2cccc81164-goog

[U-Boot] [PATCH v2 2/3] dm: core: Test uclass_first/next_device() on probe failure

[Simon Glass]

Add some tests which check the behaviour of uclass_first_device() and
uclass_next_device() when probing of a device fails.

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

Changes in v2: None

 arch/sandbox/dts/test.dts | 19 +++++++++++++
 include/dm/uclass-id.h    |  1 +
 test/dm/test-fdt.c        | 72 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index fff175d1b7..e5cdbab965 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -418,6 +418,25 @@
 			};
 		};
 	};
+
+	probing {
+		compatible = "simple-bus";
+		test1 {
+			compatible = "denx,u-boot-probe-test";
+		};
+
+		test2 {
+			compatible = "denx,u-boot-probe-test";
+		};
+
+		test3 {
+			compatible = "denx,u-boot-probe-test";
+		};
+
+		test4 {
+			compatible = "denx,u-boot-probe-test";
+		};
+	};
 };
 
 #include "sandbox_pmic.dtsi"
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 1b635e4110..316304744e 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -18,6 +18,7 @@ enum uclass_id {
 	UCLASS_TEST,
 	UCLASS_TEST_FDT,
 	UCLASS_TEST_BUS,
+	UCLASS_TEST_PROBE,
 	UCLASS_SPI_EMUL,	/* sandbox SPI device emulator */
 	UCLASS_I2C_EMUL,	/* sandbox I2C device emulator */
 	UCLASS_PCI_EMUL,	/* sandbox PCI device emulator */
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
index 3048a7b890..1770b86fed 100644
--- a/test/dm/test-fdt.c
+++ b/test/dm/test-fdt.c
@@ -12,6 +12,7 @@
 #include <asm/io.h>
 #include <dm/test.h>
 #include <dm/root.h>
+#include <dm/device-internal.h>
 #include <dm/uclass-internal.h>
 #include <dm/util.h>
 #include <test/ut.h>
@@ -99,6 +100,36 @@ UCLASS_DRIVER(testfdt) = {
 	.flags		= DM_UC_FLAG_SEQ_ALIAS,
 };
 
+struct dm_testprobe_pdata {
+	int probe_err;
+};
+
+static int testprobe_drv_probe(struct udevice *dev)
+{
+	struct dm_testprobe_pdata *pdata = dev_get_platdata(dev);
+
+	return pdata->probe_err;
+}
+
+static const struct udevice_id testprobe_ids[] = {
+	{ .compatible = "denx,u-boot-probe-test" },
+	{ }
+};
+
+U_BOOT_DRIVER(testprobe_drv) = {
+	.name	= "testprobe_drv",
+	.of_match	= testprobe_ids,
+	.id	= UCLASS_TEST_PROBE,
+	.probe	= testprobe_drv_probe,
+	.platdata_auto_alloc_size	= sizeof(struct dm_testprobe_pdata),
+};
+
+UCLASS_DRIVER(testprobe) = {
+	.name		= "testprobe",
+	.id		= UCLASS_TEST_PROBE,
+	.flags		= DM_UC_FLAG_SEQ_ALIAS,
+};
+
 int dm_check_devices(struct unit_test_state *uts, int num_devices)
 {
 	struct udevice *dev;
@@ -266,3 +297,44 @@ static int dm_test_fdt_offset(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_fdt_offset, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/**
+ * Test various error conditions with uclass_first_device() and
+ * uclass_next_device()
+ */
+static int dm_test_first_next_device(struct unit_test_state *uts)
+{
+	struct dm_testprobe_pdata *pdata;
+	struct udevice *dev, *parent = NULL;
+	int count;
+	int ret;
+
+	/* There should be 4 devices */
+	for (ret = uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0;
+	     dev;
+	     ret = uclass_next_device(&dev)) {
+		count++;
+		parent = dev_get_parent(dev);
+		}
+	ut_assertok(ret);
+	ut_asserteq(4, count);
+
+	/* Remove them and try again, with an error on the second one */
+	ut_assertok(uclass_get_device(UCLASS_TEST_PROBE, 1, &dev));
+	pdata = dev_get_platdata(dev);
+	pdata->probe_err = -ENOMEM;
+	device_remove(parent, DM_REMOVE_NORMAL);
+	ut_assertok(uclass_first_device(UCLASS_TEST_PROBE, &dev));
+	ut_asserteq(-ENOMEM, uclass_next_device(&dev));
+	ut_asserteq_ptr(dev, NULL);
+
+	/* Now an error on the first one */
+	ut_assertok(uclass_get_device(UCLASS_TEST_PROBE, 0, &dev));
+	pdata = dev_get_platdata(dev);
+	pdata->probe_err = -ENOENT;
+	device_remove(parent, DM_REMOVE_NORMAL);
+	ut_asserteq(-ENOENT, uclass_first_device(UCLASS_TEST_PROBE, &dev));
+
+	return 0;
+}
+DM_TEST(dm_test_first_next_device, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
-- 
2.12.2.816.g2cccc81164-goog

[U-Boot] [PATCH v2 3/3] dm: core: Add uclass_first/next_device_check()

[Simon Glass]

Sometimes it is useful to iterate through all devices in a uclass and
skip over those which do not work correctly (e.g fail to probe). Add two
new functions to provide this feature.

The caller must check the return value each time to make sure that the
device is valid. But the device pointer is always returned.

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

Changes in v2:
- Return even device, even those with errors
- Rename the functions

 drivers/core/uclass.c | 27 +++++++++++++++++
 include/dm/uclass.h   | 31 ++++++++++++++++++++
 test/dm/test-fdt.c    | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index 42613031ff..69456ebcb0 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -456,6 +456,33 @@ int uclass_next_device(struct udevice **devp)
 	return uclass_get_device_tail(dev, ret, devp);
 }
 
+int uclass_first_device_check(enum uclass_id id, struct udevice **devp)
+{
+	int ret;
+
+	*devp = NULL;
+	ret = uclass_find_first_device(id, devp);
+	if (ret)
+		return ret;
+	if (!*devp)
+		return 0;
+
+	return device_probe(*devp);
+}
+
+int uclass_next_device_check(struct udevice **devp)
+{
+	int ret;
+
+	ret = uclass_find_next_device(devp);
+	if (ret)
+		return ret;
+	if (!*devp)
+		return 0;
+
+	return device_probe(*devp);
+}
+
 int uclass_bind_device(struct udevice *dev)
 {
 	struct uclass *uc;
diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index 63da868ae5..4a713412a6 100644
--- a/include/dm/uclass.h
+++ b/include/dm/uclass.h
@@ -262,6 +262,37 @@ int uclass_first_device_err(enum uclass_id id, struct udevice **devp);
 int uclass_next_device(struct udevice **devp);
 
 /**
+ * uclass_first_device() - Get the first device in a uclass
+ *
+ * The device returned is probed if necessary, and ready for use
+ *
+ * This function is useful to start iterating through a list of devices which
+ * are functioning correctly and can be probed.
+ *
+ * @id: Uclass ID to look up
+ * @devp: Returns pointer to the first device in that uclass, or NULL if there
+ * is no first device
+ * @return 0 if OK (found or not found), other -ve on error. If an error occurs
+ * it is still possible to move to the next device.
+ */
+int uclass_first_device_check(enum uclass_id id, struct udevice **devp);
+
+/**
+ * uclass_next_device() - Get the next device in a uclass
+ *
+ * The device returned is probed if necessary, and ready for use
+ *
+ * This function is useful to start iterating through a list of devices which
+ * are functioning correctly and can be probed.
+ *
+ * @devp: On entry, pointer to device to lookup. On exit, returns pointer
+ * to the next device in the uclass if any
+ * @return 0 if OK (found or not found), other -ve on error. If an error occurs
+ * it is still possible to move to the next device.
+ */
+int uclass_next_device_check(struct udevice **devp);
+
+/**
  * uclass_resolve_seq() - Resolve a device's sequence number
  *
  * On entry dev->seq is -1, and dev->req_seq may be -1 (to allocate a
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
index 1770b86fed..f028fe5935 100644
--- a/test/dm/test-fdt.c
+++ b/test/dm/test-fdt.c
@@ -338,3 +338,83 @@ static int dm_test_first_next_device(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_first_next_device, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/**
+ * check_devices() - Check return values and pointers
+ *
+ * This runs through a full sequence of uclass_first_device_check()...
+ * uclass_next_device_check() checking that the return values and devices
+ * are correct.
+ *
+ * @uts: Test state
+ * @devlist: List of expected devices
+ * @mask: Indicates which devices should return an error. Device n should
+ *	  return error (-NOENT - n) if bit n is set, or no error (i.e. 0) if
+ *	  bit n is clear.
+ */
+static int check_devices(struct unit_test_state *uts,
+			 struct udevice *devlist[], int mask)
+{
+	int expected_ret;
+	struct udevice *dev;
+	int i;
+
+	expected_ret = (mask & 1) ? -ENOENT : 0;
+	mask >>= 1;
+	ut_asserteq(expected_ret,
+		    uclass_first_device_check(UCLASS_TEST_PROBE, &dev));
+	for (i = 0; i < 4; i++) {
+		ut_asserteq_ptr(devlist[i], dev);
+		expected_ret = (mask & 1) ? -ENOENT - (i + 1) : 0;
+		mask >>= 1;
+		ut_asserteq(expected_ret, uclass_next_device_check(&dev));
+	}
+	ut_asserteq_ptr(NULL, dev);
+
+	return 0;
+}
+
+/* Test uclass_first_device_check() and uclass_next_device_check() */
+static int dm_test_first_next_ok_device(struct unit_test_state *uts)
+{
+	struct dm_testprobe_pdata *pdata;
+	struct udevice *dev, *parent = NULL, *devlist[4];
+	int count;
+	int ret;
+
+	/* There should be 4 devices */
+	count = 0;
+	for (ret = uclass_first_device_check(UCLASS_TEST_PROBE, &dev);
+	     dev;
+	     ret = uclass_next_device_check(&dev)) {
+		ut_assertok(ret);
+		devlist[count++] = dev;
+		parent = dev_get_parent(dev);
+		}
+	ut_asserteq(4, count);
+	ut_assertok(uclass_first_device_check(UCLASS_TEST_PROBE, &dev));
+	ut_assertok(check_devices(uts, devlist, 0));
+
+	/* Remove them and try again, with an error on the second one */
+	pdata = dev_get_platdata(devlist[1]);
+	pdata->probe_err = -ENOENT - 1;
+	device_remove(parent, DM_REMOVE_NORMAL);
+	ut_assertok(check_devices(uts, devlist, 1 << 1));
+
+	/* Now an error on the first one */
+	pdata = dev_get_platdata(devlist[0]);
+	pdata->probe_err = -ENOENT - 0;
+	device_remove(parent, DM_REMOVE_NORMAL);
+	ut_assertok(check_devices(uts, devlist, 3 << 0));
+
+	/* Now errors on all */
+	pdata = dev_get_platdata(devlist[2]);
+	pdata->probe_err = -ENOENT - 2;
+	pdata = dev_get_platdata(devlist[3]);
+	pdata->probe_err = -ENOENT - 3;
+	device_remove(parent, DM_REMOVE_NORMAL);
+	ut_assertok(check_devices(uts, devlist, 0xf << 0));
+
+	return 0;
+}
+DM_TEST(dm_test_first_next_ok_device, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
-- 
2.12.2.816.g2cccc81164-goog

[U-Boot] [PATCH v2 3/3] dm: core: Add uclass_first/next_device_check()

[Simon Glass]

Hi,

On 23 April 2017 at 20:10, Simon Glass <sjg@chromium.org> wrote:
> Sometimes it is useful to iterate through all devices in a uclass and
> skip over those which do not work correctly (e.g fail to probe). Add two
> new functions to provide this feature.
>
> The caller must check the return value each time to make sure that the
> device is valid. But the device pointer is always returned.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Return even device, even those with errors
> - Rename the functions
>
>  drivers/core/uclass.c | 27 +++++++++++++++++
>  include/dm/uclass.h   | 31 ++++++++++++++++++++
>  test/dm/test-fdt.c    | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 138 insertions(+)


This patch has been sitting around for a while. Does anyone think it is useful?

Regards,
Simon

[U-Boot] [PATCH v2 3/3] dm: core: Add uclass_first/next_device_check()

[Mario Six]

Hi Simon,

On Fri, Jun 9, 2017 at 12:41 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 23 April 2017 at 20:10, Simon Glass <sjg@chromium.org> wrote:
>> Sometimes it is useful to iterate through all devices in a uclass and
>> skip over those which do not work correctly (e.g fail to probe). Add two
>> new functions to provide this feature.
>>
>> The caller must check the return value each time to make sure that the
>> device is valid. But the device pointer is always returned.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v2:
>> - Return even device, even those with errors
>> - Rename the functions
>>
>>  drivers/core/uclass.c | 27 +++++++++++++++++
>>  include/dm/uclass.h   | 31 ++++++++++++++++++++
>>  test/dm/test-fdt.c    | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 138 insertions(+)
>
>
> This patch has been sitting around for a while. Does anyone think it is useful?
>
> Regards,
> Simon
> _______________________________________________

I actually made a patch for one of our boards with two functions that have the
same semantics in our repository (just named uclass_{first,next}_device_skip).

The background is that on this board we have a network interface controlled by
an FPGA and another interface controlled by the SoC. The problem I faced was
that when the interfaces were brought up in net/eth-uclass.c the FPGA was not
yet running, hence the probe of the FPGA's network interface (which came first)
failed. But since the process then stopped there, the SoC's interface was not
activated either, which left the board without a network connection (which
would be a problem in production).

Aside from that, I think there are many use cases where the "skipping behavior"
would be prudent. For example in the gpio command: if any of the GPIO
controllers fail to probe for some reason, then "gpio st -a" will stop at this
particular controller, and won't show the state of any other controllers.

So, in conclusion, yes, I think it would be a nice addition.

Best regards,

Mario

[U-Boot] [PATCH v2 3/3] dm: core: Add uclass_first/next_device_check()

[Heinrich Schuchardt]

On 06/09/2017 12:41 AM, Simon Glass wrote:
> Hi,
> 
> On 23 April 2017 at 20:10, Simon Glass <sjg@chromium.org> wrote:
>> Sometimes it is useful to iterate through all devices in a uclass and
>> skip over those which do not work correctly (e.g fail to probe). Add two
>> new functions to provide this feature.
>>
>> The caller must check the return value each time to make sure that the
>> device is valid. But the device pointer is always returned.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v2:
>> - Return even device, even those with errors
>> - Rename the functions
>>
>>  drivers/core/uclass.c | 27 +++++++++++++++++
>>  include/dm/uclass.h   | 31 ++++++++++++++++++++
>>  test/dm/test-fdt.c    | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 138 insertions(+)
> 
> 
> This patch has been sitting around for a while. Does anyone think it is useful?
> 
> Regards,
> Simon
> 
Hello Simon,

the patch is needed as basis for a patch to bootefi.

Cf. https://patchwork.ozlabs.org/patch/752214/

Best regards

Heinrich

[U-Boot] [PATCH v2 3/3] dm: core: Add uclass_first/next_device_check()

[sjg at google.com]

On 06/09/2017 12:41 AM, Simon Glass wrote:
> Hi,
>
> On 23 April 2017 at 20:10, Simon Glass <sjg@chromium.org> wrote:
>> Sometimes it is useful to iterate through all devices in a uclass and
>> skip over those which do not work correctly (e.g fail to probe). Add two
>> new functions to provide this feature.
>>
>> The caller must check the return value each time to make sure that the
>> device is valid. But the device pointer is always returned.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v2:
>> - Return even device, even those with errors
>> - Rename the functions
>>
>>  drivers/core/uclass.c | 27 +++++++++++++++++
>>  include/dm/uclass.h   | 31 ++++++++++++++++++++
>>  test/dm/test-fdt.c    | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 138 insertions(+)
>
>
> This patch has been sitting around for a while. Does anyone think it is useful?
>
> Regards,
> Simon
>
Hello Simon,

the patch is needed as basis for a patch to bootefi.

Cf. https://patchwork.ozlabs.org/patch/752214/

Best regards

Heinrich



Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 1/3] dm: core: Clarify uclass_first/next_device() comments

[sjg at google.com]

These are not as clear as they could be. Tidy them up a bit. Also fix a
tiny code-style nit.

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

Changes in v2: None

 drivers/core/uclass.c |  3 +--
 include/dm/uclass.h   | 13 +++++++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 2/3] dm: core: Test uclass_first/next_device() on probe failure

[sjg at google.com]

Add some tests which check the behaviour of uclass_first_device() and
uclass_next_device() when probing of a device fails.

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

Changes in v2: None

 arch/sandbox/dts/test.dts | 19 +++++++++++++
 include/dm/uclass-id.h    |  1 +
 test/dm/test-fdt.c        | 72 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)

Applied to u-boot-dm, thanks!