[U-Boot] [PATCH 0/5] dm: core: Fix up test failures

[U-Boot] [PATCH 0/5] dm: core: Fix up test failures

[Simon Glass]

While implementing the USB tests some strange failures crept in. I was not
able to debug them at the time and they were seemingly independent of the
USB tests. I then forgot about it.

I have now tracked these failures down. This series corrects them so that
the driver model tests run from start to finish.

The main problems are:

- recursive calls to uclass device_unbind() are not handled from within
   uclass_destroy()
- dm_test_uclass_before_ready() can stop sandbox from working correctly for
   tests that run after it

This series fixes these as well as a minor LCD console problem with sandbox.
It also updates the driver model README for recently added tests.


Simon Glass (5):
  dm: core: Handle recursive unbinding of uclass devices
  dm: usb: Add a terminator to the string destructor list
  lcd: Call lcd_sync() after completing the scroll
  dm: Update the README to reflect the current test output
  dm: test: Don't clear global_data in dm_test_uclass_before_ready()

 common/lcd_console.c           |  2 +-
 doc/driver-model/README.txt    | 58 ++++++++++++++++++++++++++++++++++++------
 drivers/core/uclass.c          | 12 +++++++--
 drivers/usb/emul/sandbox_hub.c |  1 +
 test/dm/core.c                 |  6 +++--
 5 files changed, 66 insertions(+), 13 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 1/5] dm: core: Handle recursive unbinding of uclass devices

[Simon Glass]

Since a device can have children in the same uclass as itself, we need
to handle unbinding carefully: we must allow that unbinding a device in a
uclass may cause another device in the same uclass to be unbound.

Adjust the code to cope.

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

 drivers/core/uclass.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index 98c15e5..45fcd08 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -99,10 +99,18 @@ fail_mem:
 int uclass_destroy(struct uclass *uc)
 {
 	struct uclass_driver *uc_drv;
-	struct udevice *dev, *tmp;
+	struct udevice *dev;
 	int ret;
 
-	list_for_each_entry_safe(dev, tmp, &uc->dev_head, uclass_node) {
+	/*
+	 * We cannot use list_for_each_entry_safe() here. If a device in this
+	 * uclass has a child device also in this uclass, it will be also be
+	 * unbound (by the recursion in the call to device_unbind() below).
+	 * We can loop until the list is empty.
+	 */
+	while (!list_empty(&uc->dev_head)) {
+		dev = list_first_entry(&uc->dev_head, struct udevice,
+				       uclass_node);
 		ret = device_remove(dev);
 		if (ret)
 			return ret;
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 1/5] dm: core: Handle recursive unbinding of uclass devices

[Joe Hershberger]

Hi Simon,

On Sun, Apr 19, 2015 at 8:20 AM, Simon Glass <sjg@chromium.org> wrote:
> Since a device can have children in the same uclass as itself, we need
> to handle unbinding carefully: we must allow that unbinding a device in a
> uclass may cause another device in the same uclass to be unbound.
>
> Adjust the code to cope.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

This fixed the seg fault I was seeing. Thanks!

Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
Tested-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 1/5] dm: core: Handle recursive unbinding of uclass devices

[Simon Glass]

On 21 April 2015 at 09:30, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Sun, Apr 19, 2015 at 8:20 AM, Simon Glass <sjg@chromium.org> wrote:
>> Since a device can have children in the same uclass as itself, we need
>> to handle unbinding carefully: we must allow that unbinding a device in a
>> uclass may cause another device in the same uclass to be unbound.
>>
>> Adjust the code to cope.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> This fixed the seg fault I was seeing. Thanks!
>
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
> Tested-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot-dm.

[U-Boot] [PATCH 2/5] dm: usb: Add a terminator to the string destructor list

[Simon Glass]

The terminator is missing. Add it for completeness.

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

 drivers/usb/emul/sandbox_hub.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/emul/sandbox_hub.c b/drivers/usb/emul/sandbox_hub.c
index 280c708..baf8bdc 100644
--- a/drivers/usb/emul/sandbox_hub.c
+++ b/drivers/usb/emul/sandbox_hub.c
@@ -32,6 +32,7 @@ static struct usb_string hub_strings[] = {
 	{STRING_MANUFACTURER,	"sandbox"},
 	{STRING_PRODUCT,	"hub"},
 	{STRING_SERIAL,		"2345"},
+	{},
 };
 
 static struct usb_device_descriptor hub_device_desc = {
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 2/5] dm: usb: Add a terminator to the string destructor list

[Joe Hershberger]

Hi Simon,

On Sun, Apr 19, 2015 at 8:20 AM, Simon Glass <sjg@chromium.org> wrote:
> The terminator is missing. Add it for completeness.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
Tested-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 2/5] dm: usb: Add a terminator to the string destructor list

[Marek Vasut]

On Tuesday, April 21, 2015 at 05:40:00 PM, Joe Hershberger wrote:
> Hi Simon,
> 
> On Sun, Apr 19, 2015 at 8:20 AM, Simon Glass <sjg@chromium.org> wrote:
> > The terminator is missing. Add it for completeness.
> > 
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> 
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
> Tested-by: Joe Hershberger <joe.hershberger@ni.com>

Hi Simon,

will you pick these please ?

Best regards,
Marek Vasut

[U-Boot] [PATCH 2/5] dm: usb: Add a terminator to the string destructor list

[Simon Glass]

Hi Marek,

Yes, will do. I need to review the pmic stuff too.

Regards,
Simon


On 21 April 2015 at 11:08, Marek Vasut <marex@denx.de> wrote:
> On Tuesday, April 21, 2015 at 05:40:00 PM, Joe Hershberger wrote:
>> Hi Simon,
>>
>> On Sun, Apr 19, 2015 at 8:20 AM, Simon Glass <sjg@chromium.org> wrote:
>> > The terminator is missing. Add it for completeness.
>> >
>> > Signed-off-by: Simon Glass <sjg@chromium.org>
>>
>> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
>> Tested-by: Joe Hershberger <joe.hershberger@ni.com>
>
> Hi Simon,
>
> will you pick these please ?
>
> Best regards,
> Marek Vasut

[U-Boot] [PATCH 2/5] dm: usb: Add a terminator to the string destructor list

[Simon Glass]

On 21 April 2015 at 11:11, Simon Glass <sjg@chromium.org> wrote:
> Hi Marek,
>
> Yes, will do. I need to review the pmic stuff too.
>
> Regards,
> Simon
>
>
> On 21 April 2015 at 11:08, Marek Vasut <marex@denx.de> wrote:
>> On Tuesday, April 21, 2015 at 05:40:00 PM, Joe Hershberger wrote:
>>> Hi Simon,
>>>
>>> On Sun, Apr 19, 2015 at 8:20 AM, Simon Glass <sjg@chromium.org> wrote:
>>> > The terminator is missing. Add it for completeness.
>>> >
>>> > Signed-off-by: Simon Glass <sjg@chromium.org>
>>>
>>> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
>>> Tested-by: Joe Hershberger <joe.hershberger@ni.com>
>>
>> Hi Simon,
>>
>> will you pick these please ?
>>
>> Best regards,
>> Marek Vasut

Applied to u-boot-dm.

[U-Boot] [PATCH 3/5] lcd: Call lcd_sync() after completing the scroll

[Simon Glass]

On sandbox, if you add a printf() to malloc() for debugging, the output will
eventually cause the screen to scroll. Since lcd_sync() calls SDL functions
which allocate memory, and this happens before we have updated
console_curr_row, U-Boot gets locked in an infinite loop.

Flip the order of the two statements to avoid this.

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

 common/lcd_console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lcd_console.c b/common/lcd_console.c
index 8bf83b9..ab48fd6 100644
--- a/common/lcd_console.c
+++ b/common/lcd_console.c
@@ -120,8 +120,8 @@ static void console_scrollup(void)
 		*ppix++ = bg_color;
 	}
 #endif
-	lcd_sync();
 	console_curr_row -= rows;
+	lcd_sync();
 }
 
 static inline void console_back(void)
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 3/5] lcd: Call lcd_sync() after completing the scroll

[Simon Glass]

Hi,

On 19 April 2015 at 07:21, Simon Glass <sjg@chromium.org> wrote:
> On sandbox, if you add a printf() to malloc() for debugging, the output will
> eventually cause the screen to scroll. Since lcd_sync() calls SDL functions
> which allocate memory, and this happens before we have updated
> console_curr_row, U-Boot gets locked in an infinite loop.
>
> Flip the order of the two statements to avoid this.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  common/lcd_console.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/lcd_console.c b/common/lcd_console.c
> index 8bf83b9..ab48fd6 100644
> --- a/common/lcd_console.c
> +++ b/common/lcd_console.c
> @@ -120,8 +120,8 @@ static void console_scrollup(void)
>                 *ppix++ = bg_color;
>         }
>  #endif
> -       lcd_sync();
>         console_curr_row -= rows;
> +       lcd_sync();
>  }
>
>  static inline void console_back(void)
> --
> 2.2.0.rc0.207.ga3a616c
>

This patch is obsolete now since this problem is fixed by an earlier
patch, now applied.

I am dropping it.

Regards,
Simon

[U-Boot] [PATCH 4/5] dm: Update the README to reflect the current test output

[Simon Glass]

There are a lot more tests now. To avoid confusion add the updated test
output to the driver model README.

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

 doc/driver-model/README.txt | 58 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt
index f83264d..f0276b1 100644
--- a/doc/driver-model/README.txt
+++ b/doc/driver-model/README.txt
@@ -95,43 +95,82 @@ are provided in test/dm. To run them, try:
 You should see something like this:
 
     <...U-Boot banner...>
-    Running 29 driver model tests
+    Running 53 driver model tests
     Test: dm_test_autobind
     Test: dm_test_autoprobe
+    Test: dm_test_bus_child_post_bind
+    Test: dm_test_bus_child_post_bind_uclass
+    Test: dm_test_bus_child_pre_probe_uclass
     Test: dm_test_bus_children
-    Device 'd-test': seq 3 is in use by 'b-test'
-    Device 'c-test at 0': seq 0 is in use by 'a-test'
-    Device 'c-test at 1': seq 1 is in use by 'd-test'
+    Device 'c-test at 0': seq 0 is in use by 'd-test'
+    Device 'c-test at 1': seq 1 is in use by 'f-test'
     Test: dm_test_bus_children_funcs
     Test: dm_test_bus_children_iterators
     Test: dm_test_bus_parent_data
+    Test: dm_test_bus_parent_data_uclass
     Test: dm_test_bus_parent_ops
+    Test: dm_test_bus_parent_platdata
+    Test: dm_test_bus_parent_platdata_uclass
     Test: dm_test_children
+    Test: dm_test_device_get_uclass_id
+    Test: dm_test_eth
+    Using eth at 10002000 device
+    Using eth at 10003000 device
+    Using eth at 10004000 device
+    Test: dm_test_eth_alias
+    Using eth at 10002000 device
+    Using eth at 10004000 device
+    Using eth at 10002000 device
+    Using eth at 10003000 device
+    Test: dm_test_eth_prime
+    Using eth at 10003000 device
+    Using eth at 10002000 device
+    Test: dm_test_eth_rotate
+
+    Error: eth at 10004000 address not set.
+
+    Error: eth at 10004000 address not set.
+    Using eth at 10002000 device
+
+    Error: eth at 10004000 address not set.
+
+    Error: eth at 10004000 address not set.
+    Using eth at 10004000 device
     Test: dm_test_fdt
-    Device 'd-test': seq 3 is in use by 'b-test'
     Test: dm_test_fdt_offset
     Test: dm_test_fdt_pre_reloc
     Test: dm_test_fdt_uclass_seq
-    Device 'd-test': seq 3 is in use by 'b-test'
-    Device 'a-test': seq 0 is in use by 'd-test'
     Test: dm_test_gpio
     extra-gpios: get_value: error: gpio b5 not reserved
     Test: dm_test_gpio_anon
     Test: dm_test_gpio_copy
     Test: dm_test_gpio_leak
     extra-gpios: get_value: error: gpio b5 not reserved
+    Test: dm_test_gpio_phandles
     Test: dm_test_gpio_requestf
+    Test: dm_test_i2c_bytewise
+    Test: dm_test_i2c_find
+    Test: dm_test_i2c_offset
+    Test: dm_test_i2c_offset_len
+    Test: dm_test_i2c_probe_empty
+    Test: dm_test_i2c_read_write
+    Test: dm_test_i2c_speed
     Test: dm_test_leak
     Test: dm_test_lifecycle
+    Test: dm_test_net_retry
+    Using eth at 10004000 device
+    Using eth at 10002000 device
+    Using eth at 10004000 device
     Test: dm_test_operations
     Test: dm_test_ordering
+    Test: dm_test_pci_base
+    Test: dm_test_pci_swapcase
     Test: dm_test_platdata
     Test: dm_test_pre_reloc
     Test: dm_test_remove
     Test: dm_test_spi_find
     Invalid chip select 0:0 (err=-19)
     SF: Failed to get idcodes
-    Device 'name-emul': seq 0 is in use by 'name-emul'
     SF: Detected M25P16 with page size 256 Bytes, erase size 64 KiB, total 2 MiB
     Test: dm_test_spi_flash
     2097152 bytes written in 0 ms
@@ -150,6 +189,9 @@ You should see something like this:
     SF: Detected M25P16 with page size 256 Bytes, erase size 64 KiB, total 2 MiB
     Test: dm_test_uclass
     Test: dm_test_uclass_before_ready
+    Test: dm_test_usb_base
+    Test: dm_test_usb_flash
+    USB-1:   scanning bus 1 for devices... 2 USB Device(s) found
     Failures: 0
 
 
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 4/5] dm: Update the README to reflect the current test output

[Joe Hershberger]

Hi Simon,

On Sun, Apr 19, 2015 at 8:21 AM, Simon Glass <sjg@chromium.org> wrote:
> There are a lot more tests now. To avoid confusion add the updated test
> output to the driver model README.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---

Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 4/5] dm: Update the README to reflect the current test output

[Simon Glass]

On 21 April 2015 at 09:40, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Sun, Apr 19, 2015 at 8:21 AM, Simon Glass <sjg@chromium.org> wrote:
>> There are a lot more tests now. To avoid confusion add the updated test
>> output to the driver model README.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot-dm.

[U-Boot] [PATCH 5/5] dm: test: Don't clear global_data in dm_test_uclass_before_ready()

[Simon Glass]

We must not clear global_data even in tests, since the ram_buffer (which
is used by malloc()) will also be lost, and subsequent tests will fail.

Zero only the global_data fields that are required for the test to function.

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

 test/dm/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/test/dm/core.c b/test/dm/core.c
index 990d390..4b9c987 100644
--- a/test/dm/core.c
+++ b/test/dm/core.c
@@ -596,12 +596,14 @@ static int dm_test_uclass_before_ready(struct dm_test_state *dms)
 
 	ut_assertok(uclass_get(UCLASS_TEST, &uc));
 
-	memset(gd, '\0', sizeof(*gd));
+	gd->dm_root = NULL;
+	gd->dm_root_f = NULL;
+	memset(&gd->uclass_root, '\0', sizeof(gd->uclass_root));
+
 	ut_asserteq_ptr(NULL, uclass_find(UCLASS_TEST));
 
 	return 0;
 }
-
 DM_TEST(dm_test_uclass_before_ready, 0);
 
 static int dm_test_device_get_uclass_id(struct dm_test_state *dms)
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 5/5] dm: test: Don't clear global_data in dm_test_uclass_before_ready()

[Joe Hershberger]

Hi Simon,

On Sun, Apr 19, 2015 at 8:21 AM, Simon Glass <sjg@chromium.org> wrote:
> We must not clear global_data even in tests, since the ram_buffer (which
> is used by malloc()) will also be lost, and subsequent tests will fail.
>
> Zero only the global_data fields that are required for the test to function.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

For me, this fixed:

"""
Test: dm_test_usb_base
/home/joe/u-boot/test/dm/test-main.c:27, dm_test_init(): 0 ==
dm_init(): Expected 0, got -12
/home/joe/u-boot/test/dm/test-main.c:93, dm_test_main(): 0 ==
dm_test_init(dms): Expected 0, got -1
"""

Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
Tested-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 5/5] dm: test: Don't clear global_data in dm_test_uclass_before_ready()

[Simon Glass]

On 21 April 2015 at 10:00, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Sun, Apr 19, 2015 at 8:21 AM, Simon Glass <sjg@chromium.org> wrote:
>> We must not clear global_data even in tests, since the ram_buffer (which
>> is used by malloc()) will also be lost, and subsequent tests will fail.
>>
>> Zero only the global_data fields that are required for the test to function.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> For me, this fixed:
>
> """
> Test: dm_test_usb_base
> /home/joe/u-boot/test/dm/test-main.c:27, dm_test_init(): 0 ==
> dm_init(): Expected 0, got -12
> /home/joe/u-boot/test/dm/test-main.c:93, dm_test_main(): 0 ==
> dm_test_init(dms): Expected 0, got -1
> """
>
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
> Tested-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot-dm.