Hi Michal,
On Sat, 24 Sept 2022 at 14:10, Michal Suchánek msuchanek@suse.de wrote:
Hello,
On Sat, Sep 17, 2022 at 07:04:25PM +0200, Michal Suchánek wrote:
Hello,
On Sat, Sep 17, 2022 at 09:02:53AM -0600, Simon Glass wrote:
Hi Michal,
On Wed, 31 Aug 2022 at 11:44, Simon Glass sjg@chromium.org wrote:
Hi Michal,
On Wed, 31 Aug 2022 at 01:39, Michal Suchánek msuchanek@suse.de wrote:
Hello,
On Tue, Aug 30, 2022 at 09:15:12PM -0600, Simon Glass wrote:
Hi Michal,
On Tue, 30 Aug 2022 at 10:48, Michal Suchánek msuchanek@suse.de wrote: > > On Tue, Aug 30, 2022 at 09:56:52AM -0600, Simon Glass wrote: > > Hi Michal, > > > > On Tue, 30 Aug 2022 at 04:23, Michal Suchánek msuchanek@suse.de wrote: > > > > > > On Sat, Aug 27, 2022 at 07:52:27PM -0600, Simon Glass wrote: > > > > Hi Michal, > > > > > > > > On Fri, 19 Aug 2022 at 14:23, Michal Suchanek msuchanek@suse.de wrote: > > > > > > > > > > When probing a device fails NULL pointer is returned, and other devices > > > > > cannot be iterated. Skip to next device on error instead. > > > > > > > > > > Fixes: 6494d708bf ("dm: Add base driver model support") > > > > > > > > I think you should drop this as you are doing a change of behaviour, > > > > not fixing a bug! > > > > > > You can hardly fix a bug without a change in behavior. > > > > > > These functions are used for iterating devices, and are not iterating > > > devices. That's clearly a bug. > > > > If it were clear I would have changed this long ago. The new way you > > have this function ignores errors, so they cannot be reported. > > > > We should almost always report errors, which is why I think your > > methods should be named differently. > > > > > > > > > > Signed-off-by: Michal Suchanek msuchanek@suse.de > > > > > --- > > > > > v2: - Fix up tests > > > > > v3: - Fix up API doc > > > > > - Correctly forward error from uclass_get > > > > > - Do not return an error when last device fails to probe > > > > > - Drop redundant initialization > > > > > - Wrap at 80 columns > > > > > --- > > > > > drivers/core/uclass.c | 32 ++++++++++++++++++++++++-------- > > > > > include/dm/uclass.h | 13 ++++++++----- > > > > > test/dm/test-fdt.c | 20 ++++++++++++++++---- > > > > > 3 files changed, 48 insertions(+), 17 deletions(-) > > > > > > > > Unfortunately this still fails one test. Try 'make qcheck' to see it - > > > > it is ethernet. > > > > > > I will look at that.
How do you debug test failures?
There is indeed a test that regresses.
However, when I run
ping 1.1.1.1
I get to see the printfs that I added to net_loop
when I run
ut dm dm_test_eth_act
u-boot crashes, and no printf output is seen, not even the print that should report entering net_loop. Given that the assert that is reported as failing is test/dm/eth.c:133, dm_test_eth_act(): -ENODEV == net_loop(PING): Expected 0xffffffed (-19), got 0x0 (0) it should run the net_loop to get that error.
It's nice that we have tests but if they cannot be debugged they are not all that useful.
Why don't you try gdb?
This is part of the setup script I use:
function rt_get_suite_and_name() { local arg="$1" #echo arg $arg suite= name=
# The symbol is something like this: # _u_boot_list_2_ut_bootstd_test_2_vbe_simple_test_base # Split it into the suite name (bootstd) and test name # (vbe_simple_test_base) read suite name < \ <(nm /tmp/b/$exec/u-boot |grep "list_2_ut.*$arg.*" \ | cut -d' ' -f3 \ | head -1 \ | sed -n 's/_u_boot_list_2_ut_(.*)_test_2_/\1 /p') #echo suite $suite #echo name $name #name=${1#dm_test_} #name=${name#ut_dm_} }
# Run a test function rt() { local exec=sandbox local suite name rt_get_suite_and_name $1 $exec
/tmp/b/$exec/u-boot -T $2 -c "ut $suite $name" }
# Run a test verbosely function rtv() { local exec=sandbox local suite name rt_get_suite_and_name $1 $exec
/tmp/b/$exec/u-boot -v -T $2 -c "ut $suite $name" }
# Run a test in the debugger function rtd() { local exec=sandbox local suite name rt_get_suite_and_name $1 $exec
gdb-multiarch --args /tmp/b/$exec/u-boot -T $2 -c "ut $suite $name" }
# Run a test in the debugger verbosely function rtdv() { local exec=sandbox local suite name rt_get_suite_and_name $1 $exec
gdb-multiarch --args /tmp/b/$exec/u-boot -T $2 -v -c "ut $suite $name" }
So you can use 'rtdv dm_test_eth_act' for example.
Regards, Simon