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 E2829C433F5 for ; Thu, 29 Sep 2022 10:04:17 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1BD7C84D9E; Thu, 29 Sep 2022 12:02:44 +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="I+YZsm65"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id A3E1784C81; Thu, 29 Sep 2022 12:01:29 +0200 (CEST) Received: from mail-ot1-x332.google.com (mail-ot1-x332.google.com [IPv6:2607:f8b0:4864:20::332]) (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 93C3084CB9 for ; Thu, 29 Sep 2022 12:01:16 +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-ot1-x332.google.com with SMTP id cy15-20020a056830698f00b0065c530585afso575355otb.2 for ; Thu, 29 Sep 2022 03:01:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=F6KE9eJEbS2xk6XHrKLhH7rvzpH4ZOOUwD77UDCFLOA=; b=I+YZsm65we1U/NJyQIHkZyh+F6X4jkDLARUh1KjawP+PWwgQEQDBqeAVCNx6ajR8xr tUkyNZa+G89i0OCaoo7yXA0tmBrhsQShP4VfssqP13/mpP+C3oC1k+TKj5mH/cftnKmN KjJ8j/5kSJFyKP5uDAPvwD5xsiwon1nvhXdlY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=F6KE9eJEbS2xk6XHrKLhH7rvzpH4ZOOUwD77UDCFLOA=; b=TTqEZ8ICiejs1q2m5lX9zE1Vfao/Y2hMzZkSpjru79QT0GnuAngOf5pZdNzFUWmTdp ILXVlStfE32f4JXgWwmjCLUK6nnvJ5X4w6G21jbXwloGLwgfCFldqzoaggB3lwilfbrt +uWZsMwwBsy3mIAHj3s8Zc6/sXRqLZLOAV+Mp10n5Sx85ZJUoQJ8rkTSVIwUrdgLy9JY 4AOaKlSVIoZhc505i1qC1n9tJYcbke9C8l4E9cyWhP0xCCyl3j2SRXT4vaTKGe/uhqFU bxHcd3RQZYmw5pDYVoIE4SV5HeonlVKe1kSEBqIpftWoTwdmDZJwXWrxQA86JjM5TSGR TlKw== X-Gm-Message-State: ACrzQf0uKyTbcjJw1cHzKKG49FAXP3dRZKt3GzJHZAtqkBm7Em0l7MWT rINs11xHJ3ObL14OO3iE3m6k42Tu+fTshIq06HfpUA== X-Google-Smtp-Source: AMsMyM6il0JI9Dxwp8YjZDSsH1rAuh9v/CSOjMqlQHF+F35j+rYOIOEdhdCihfIH6Ewc/bYZ5Mow7PGlhSEump77hd0= X-Received: by 2002:a05:6830:20d3:b0:655:cd22:b47c with SMTP id z19-20020a05683020d300b00655cd22b47cmr922398otq.351.1664445672337; Thu, 29 Sep 2022 03:01:12 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Simon Glass Date: Thu, 29 Sep 2022 04:00:53 -0600 Message-ID: Subject: Re: [PATCH v5 15/15] dm: core: Do not stop uclass iteration on error To: Michal Suchanek Cc: U-Boot Mailing List , Tomas Hlavacek , =?UTF-8?B?VmlrdG9yIEvFmWl2w6Fr?= , Pavel Herrmann , Marek Vasut Content-Type: text/plain; charset="UTF-8" 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 Tue, 27 Sept 2022 at 15:38, Michal Suchanek wrote: > > When probing a device fails NULL pointer is returned, and following > devices in uclass list cannot be iterated. Skip to next device on error > instead. > > With that the only condition under which these simple iteration > functions return error is when the dm is not initialized at uclass_get > time. This is not all that interesting, change return type to void. > > Fixes: 6494d708bf ("dm: Add base driver model support") > 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 > v4: - change return value to void > - further simplify iteration > --- > drivers/core/uclass.c | 30 ++++++++++++++++++------------ > include/dm/uclass.h | 13 ++++++------- > test/dm/core.c | 10 ++++------ > test/dm/test-fdt.c | 27 ++++++++++++++++++++------- > 4 files changed, 48 insertions(+), 32 deletions(-) > Reviewed-by: Simon Glass > diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c > index b7d11bdd23..6dec6a3973 100644 > --- a/drivers/core/uclass.c > +++ b/drivers/core/uclass.c > @@ -574,28 +574,34 @@ int uclass_get_device_by_phandle(enum uclass_id id, struct udevice *parent, > } > #endif > > -int uclass_first_device(enum uclass_id id, struct udevice **devp) > +/* > + * Starting from the given device return first device in the uclass that device, return the first > + * probes successfully. Please describe the args here. > + */ > +static void _uclass_next_device(struct udevice *dev, struct udevice **devp) > +{ > + for (; dev; uclass_find_next_device(&dev)) { > + if (!device_probe(dev)) > + break; > + } > + *devp = dev; > +} > + > +void uclass_first_device(enum uclass_id id, struct udevice **devp) > { > struct udevice *dev; > int ret; > > - *devp = NULL; > ret = uclass_find_first_device(id, &dev); > - if (!dev) > - return 0; > - return uclass_get_device_tail(dev, ret, devp); > + _uclass_next_device(dev, devp); > } > > -int uclass_next_device(struct udevice **devp) > +void uclass_next_device(struct udevice **devp) > { > struct udevice *dev = *devp; > - int ret; > > - *devp = NULL; > - ret = uclass_find_next_device(&dev); > - if (!dev) > - return 0; > - return uclass_get_device_tail(dev, ret, devp); > + uclass_find_next_device(&dev); > + _uclass_next_device(dev, devp); > } > > int uclass_first_device_err(enum uclass_id id, struct udevice **devp) > diff --git a/include/dm/uclass.h b/include/dm/uclass.h > index b1c016ef9f..ee15c92063 100644 > --- a/include/dm/uclass.h > +++ b/include/dm/uclass.h > @@ -320,32 +320,31 @@ int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv, > * uclass_first_device() - Get the first device in a uclass > * > * The device returned is probed if necessary, and ready for use > + * Devices that fail to probe are skipped > * > * 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 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 > + * occurred, or NULL if there is no usable device > */ > -int uclass_first_device(enum uclass_id id, struct udevice **devp); > +void uclass_first_device(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 > + * Devices that fail to probe are skipped > * > * This function is useful to iterate 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 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 > + * no next device > */ > -int uclass_next_device(struct udevice **devp); > +void uclass_next_device(struct udevice **devp); > > /** > * uclass_first_device_err() - Get the first device in a uclass > diff --git a/test/dm/core.c b/test/dm/core.c > index 84eb76ed5f..7f3f8d183b 100644 > --- a/test/dm/core.c > +++ b/test/dm/core.c > @@ -1078,11 +1078,10 @@ static int dm_test_uclass_devices_get(struct unit_test_state *uts) > struct udevice *dev; > int ret; > > - for (ret = uclass_first_device(UCLASS_TEST, &dev); > + for (ret = uclass_first_device_check(UCLASS_TEST, &dev); > dev; > - ret = uclass_next_device(&dev)) { > + ret = uclass_next_device_check(&dev)) { > ut_assert(!ret); > - ut_assert(dev); > ut_assert(device_active(dev)); > } > > @@ -1112,11 +1111,10 @@ static int dm_test_uclass_devices_get_by_name(struct unit_test_state *uts) > * this will fail on checking condition: testdev == finddev, since the > * uclass_get_device_by_name(), returns the first device by given name. > */ > - for (ret = uclass_first_device(UCLASS_TEST_FDT, &testdev); > + for (ret = uclass_first_device_check(UCLASS_TEST_FDT, &testdev); > testdev; > - ret = uclass_next_device(&testdev)) { > + ret = uclass_next_device_check(&testdev)) { > ut_assertok(ret); > - ut_assert(testdev); > ut_assert(device_active(testdev)); > > findret = uclass_get_device_by_name(UCLASS_TEST_FDT, > diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c > index 6118ad42ca..7cbc595c51 100644 > --- a/test/dm/test-fdt.c > +++ b/test/dm/test-fdt.c > @@ -403,13 +403,12 @@ static int dm_test_first_next_device(struct unit_test_state *uts) > int ret; > > /* There should be 4 devices */ > - for (ret = uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0; > + for (uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0; > dev; > - ret = uclass_next_device(&dev)) { > + 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 */ > @@ -417,16 +416,30 @@ static int dm_test_first_next_device(struct unit_test_state *uts) > pdata = dev_get_plat(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); > + for (ret = uclass_first_device_check(UCLASS_TEST_PROBE, &dev), > + count = 0; > + dev; > + ret = uclass_next_device_check(&dev)) { > + if (!ret) > + count++; > + else > + ut_asserteq(-ENOMEM, ret); > + parent = dev_get_parent(dev); > + } > + ut_asserteq(3, count); > > /* Now an error on the first one */ > ut_assertok(uclass_get_device(UCLASS_TEST_PROBE, 0, &dev)); > pdata = dev_get_plat(dev); > pdata->probe_err = -ENOENT; > device_remove(parent, DM_REMOVE_NORMAL); > - ut_asserteq(-ENOENT, uclass_first_device(UCLASS_TEST_PROBE, &dev)); > + for (uclass_first_device(UCLASS_TEST_PROBE, &dev), count = 0; > + dev; > + uclass_next_device(&dev)) { > + count++; > + parent = dev_get_parent(dev); > + } > + ut_asserteq(2, count); > > return 0; > } > -- > 2.37.3 >