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 231AEECAAD1 for ; Tue, 30 Aug 2022 15:57:24 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 8058A8499A; Tue, 30 Aug 2022 17:57:14 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org 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=chromium.org header.i=@chromium.org header.b="JO4oHikr"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 2579B8498C; Tue, 30 Aug 2022 17:57:09 +0200 (CEST) Received: from mail-yb1-xb29.google.com (mail-yb1-xb29.google.com [IPv6:2607:f8b0:4864:20::b29]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 9B70E84986 for ; Tue, 30 Aug 2022 17:57:05 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-yb1-xb29.google.com with SMTP id p204so150049yba.3 for ; Tue, 30 Aug 2022 08:57:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc; bh=RPHndDn1PAZ4tmUWkTEW/8DXTNm/UeWkH5ILBBvoclw=; b=JO4oHikruo+PP5raOZr1UinrQ/1ZLTxbK1D+VRlRfg0/ObqA4L7lzMgaUcsCPek2So ReUaxMxGZCzZkl066/L03Fvay0qhULL7iBgAIYCzI7pMgHdN8oikOLHIvUFd2RIGBSIC 6L0le/5tM2CGQX7CIO/M3+T2JQ0W25XF2/2hM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc; bh=RPHndDn1PAZ4tmUWkTEW/8DXTNm/UeWkH5ILBBvoclw=; b=Wa+jzIlp9tSw3n/rB/kU6EWjfcKlX03tQQQZ3YsOd67vfREO8xDU64qrVmafT7FZBn 3toBpZoN2O9euMuNTLIDZzd0E5yJp02lzBTkX6/NC5hbFY/8gYNSIU3vyGCixDkKqoaj ul2qR2cywxwNbKRYwbl7mifWq6W5/3DhsFvOy1neXie7br/1KiZmvfCwTL4BFLXd7oTp HH3HRse7wKjO1IghRaQGG18rc/ucAVVtkMf4yCI3sx8aAoy7/efTr59dye4RZ0C9Ms7T T1h5xfDgE9mamOoBtpuwxXy96vT0SJNOFKlf3PR1Rzl0EWuqBMNEtnDbz6pNPWn2eSQ9 /whg== X-Gm-Message-State: ACgBeo30ATqC51TXWEL/XCtSzO40XKYbHBaR5TkHFNmeShWzgEdLDxo8 22erT9NuKwXUv9NlWVrCICl51GmFHk0k97SKPXb1uQ== X-Google-Smtp-Source: AA6agR562nECzgMzIVRAwpzjvzfoKDBQBHqkXSDAtSMcVz3iHj1EjKeBm0SCmlh7hAFF1/NCm5X5ByEW8BD4JBFIgaY= X-Received: by 2002:a25:b8c6:0:b0:692:af14:6f99 with SMTP id g6-20020a25b8c6000000b00692af146f99mr13002475ybm.197.1661875024038; Tue, 30 Aug 2022 08:57:04 -0700 (PDT) MIME-Version: 1.0 References: <20220818194640.GC28810@kitsune.suse.cz> <20220819202322.25701-1-msuchanek@suse.de> <20220830102303.GO28810@kitsune.suse.cz> In-Reply-To: <20220830102303.GO28810@kitsune.suse.cz> From: Simon Glass Date: Tue, 30 Aug 2022 09:56:52 -0600 Message-ID: Subject: Re: [PATCH v3] dm: core: Do not stop uclass iteration on error To: =?UTF-8?Q?Michal_Such=C3=A1nek?= Cc: U-Boot Mailing List , Marek Vasut , =?UTF-8?B?VmlrdG9yIEvFmWl2w6Fr?= , Pavel Herrmann , Tomas Hlavacek Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Hi Michal, On Tue, 30 Aug 2022 at 04:23, Michal Such=C3=A1nek wrot= e: > > 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 devic= es > > > 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. > > > I actually think you should create new functions for this feature, > > e.g.uclass_first_device_ok(), since it makes it impossible to see what > > when wrong with a device in the middle. > > > > I have long had all this in my mind. One idea for a future change is > > to return the error, but set dev, so that the caller knows there is a > > device, which failed. When we are at the end, dev is set to NULL. > > We already have uclass_first_device_check() and > uclass_next_device_check() to iterate all devices, including broken > ones, and getting the errors as well. > > That's for the case you want all the details, and these are for the case > you just want to get devices and don't care about the details. > > That's AFAICT as much as this iteration interface can provide, and we > have both cases covered. I see three cases: - want to see the next device, returning the error if it cannot be probed - uclass_first_device() - want to get the next device that can successfully probe - your new functi= ons - want to see each device, along with any errors - uclass_first_device_chec= k() Regards, Simon