From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8E663C04A95 for ; Sun, 25 Sep 2022 16:38:45 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id F23FF84BE3; Sun, 25 Sep 2022 18:38:42 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.b="tYXiI19d"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="D85YKqLi"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id B88C384BE3; Sun, 25 Sep 2022 18:38:40 +0200 (CEST) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 8E3E284AF7 for ; Sun, 25 Sep 2022 18:38:37 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=msuchanek@suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 537BB21C50; Sun, 25 Sep 2022 16:38:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1664123917; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MF6/x6H7jeuHqfF4GvL1kbzsbNYmWqWdiCUHsEhK7SY=; b=tYXiI19deL0MYS/mtu/rOTRfWioShyEOHLsoZLHEDIjQ9pUcPm+DSV8JGsvS8nYHEmQ9Qi uzz8VaQjRCMVS3p6p1qzGcnc+IKFVPPfLdcufUoYx8An36KIVfhbD00WXhuPZt+Ya+VM+9 0NvV1fhuAxdkDlZCiCMfMjUOUgi2yLM= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1664123917; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MF6/x6H7jeuHqfF4GvL1kbzsbNYmWqWdiCUHsEhK7SY=; b=D85YKqLigyOdYKhsRtS9uBJ6l7bkMFRsYrd4PgyrZXynUmA9ve97se7EkzIdWbbx0tgeKH ujKBIMgr7yJHwcBg== Received: from kitsune.suse.cz (kitsune.suse.cz [10.100.12.127]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 2803F2C17B; Sun, 25 Sep 2022 16:38:37 +0000 (UTC) Date: Sun, 25 Sep 2022 18:38:35 +0200 From: Michal =?iso-8859-1?Q?Such=E1nek?= To: Simon Glass Cc: U-Boot Mailing List , Marek Vasut , Viktor =?utf-8?B?S8WZaXbDoWs=?= , Pavel Herrmann , Tomas Hlavacek Subject: Re: [PATCH v3] dm: core: Do not stop uclass iteration on error Message-ID: <20220925163835.GC28810@kitsune.suse.cz> References: <20220830102303.GO28810@kitsune.suse.cz> <20220830164810.GP28810@kitsune.suse.cz> <20220831073903.GQ28810@kitsune.suse.cz> <20220917170425.GO28810@kitsune.suse.cz> <20220924200957.GB28810@kitsune.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Sun, Sep 25, 2022 at 08:15:31AM -0600, Simon Glass wrote: > Hi Michal, > > On Sat, 24 Sept 2022 at 14:10, Michal Suchánek 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 wrote: > > > > > > > > > > Hi Michal, > > > > > > > > > > On Wed, 31 Aug 2022 at 01:39, Michal Suchánek 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 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 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 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 > > > > > > > > > > > > --- > > > > > > > > > > > > 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? That's certainly an option for sandbox. However, I wonder what's the utility of hiding test output by default? I can run non-test commands in the sandbox, and they produce output as normal, and the tests don't. I even found that this default is disabled by the automatedte tests. Whent running qcheck the debug prints would be shown in the qcheck output for the failing test. So the question is who and when would want to hide the test output? Thanks Michal