All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michal Suchánek" <msuchanek@suse.de>
To: Simon Glass <sjg@chromium.org>
Cc: "U-Boot Mailing List" <u-boot@lists.denx.de>,
	"Marek Vasut" <marex@denx.de>,
	"Viktor Křivák" <viktor.krivak@gmail.com>,
	"Pavel Herrmann" <morpheus.ibis@gmail.com>,
	"Tomas Hlavacek" <tmshlvck@gmail.com>
Subject: Re: [PATCH v3] dm: core: Do not stop uclass iteration on error
Date: Tue, 30 Aug 2022 12:23:03 +0200	[thread overview]
Message-ID: <20220830102303.GO28810@kitsune.suse.cz> (raw)
In-Reply-To: <CAPnjgZ1Nz9B5+f62S+oTvegScbhp0+iCvHu7auYYe+Nu5n8eCQ@mail.gmail.com>

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.

> > 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.

> 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.

Thanks

Michal

  reply	other threads:[~2022-08-30 10:23 UTC|newest]

Thread overview: 133+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-04 17:59 [PATCH] dm: core: Do not stop uclass iteration on error Michal Suchanek
2022-08-04 19:22 ` Simon Glass
2022-08-04 19:36   ` Michal Suchánek
2022-08-04 20:30     ` Simon Glass
2022-08-17  8:27       ` [PATCH v2] " Michal Suchanek
2022-08-18 17:49         ` Simon Glass
2022-08-18 19:46           ` Michal Suchánek
2022-08-19 20:23             ` [PATCH v3] " Michal Suchanek
2022-08-28  1:52               ` Simon Glass
2022-08-30 10:23                 ` Michal Suchánek [this message]
2022-08-30 15:56                   ` Simon Glass
2022-08-30 16:48                     ` Michal Suchánek
2022-08-31  3:15                       ` Simon Glass
2022-08-31  7:39                         ` Michal Suchánek
2022-08-31 17:44                           ` Simon Glass
2022-09-17 15:02                             ` Simon Glass
2022-09-17 17:04                               ` Michal Suchánek
2022-09-24 20:09                                 ` Michal Suchánek
2022-09-25 14:15                                   ` Simon Glass
2022-09-25 16:38                                     ` Michal Suchánek
2022-09-25  8:27                               ` [PATCH v4 00/21] " Michal Suchanek
2022-09-25  8:27                                 ` [PATCH v4 01/21] dm: pci: Fix doc typo first -> next Michal Suchanek
2022-09-29  9:59                                   ` Simon Glass
2022-10-22  1:06                                     ` Simon Glass
2022-09-25  8:27                                 ` [PATCH v4 02/21] dm: core: Add note about device_probe idempotence Michal Suchanek
2022-09-29  9:59                                   ` Simon Glass
2022-09-25  8:27                                 ` [PATCH v4 03/21] dm: core: Document return value of device bind functions Michal Suchanek
2022-09-29  9:59                                   ` Simon Glass
2022-09-25  8:27                                 ` [PATCH v4 04/21] dm: blk: Add probe in blk_first_device/blk_next_device Michal Suchanek
2022-09-29 10:00                                   ` Simon Glass
2022-10-02 19:34                                     ` Michal Suchánek
2022-10-03  1:10                                       ` Simon Glass
2022-10-10 19:49                                         ` Michal Suchánek
2022-10-10 21:33                                           ` Michal Suchánek
2022-10-10 22:33                                             ` Simon Glass
2022-11-10  2:15                                               ` Simon Glass
2022-09-25  8:27                                 ` [PATCH v4 05/21] dm: core: Fix uclass_probe_all to really probe all devices Michal Suchanek
2022-09-25  8:27                                 ` [PATCH v4 06/21] dm: treewide: Do not opencode uclass_probe_all() Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 07/21] dm: pci: Fix device PCI iteration Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 08/21] bootstd: Fix listing boot devices Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 09/21] usb: ether: Fix error handling in usb_ether_init Michal Suchanek
2022-09-29 10:00                                   ` Simon Glass
2022-09-25  8:28                                 ` [PATCH v4 10/21] stdio: Fix class iteration in stdio_add_devices() Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 11/21] video: ipuv3: Fix error handling when getting the display Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 12/21] w1: Fix bus counting in w1_get_bus Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 13/21] w1: Clean up device iteration in w1_bus_find_dev Michal Suchanek
2022-09-29 10:00                                   ` Simon Glass
2022-09-25  8:28                                 ` [PATCH v4 14/21] dma: Eliminate unused variable in dma_get_cfg() Michal Suchanek
2022-09-29 10:00                                   ` Simon Glass
2022-09-25  8:28                                 ` [PATCH v4 15/21] cmd: List all uclass devices regardless of probe error Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 16/21] dm: treewide: Use uclass_first_device_err when accessing one device Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 17/21] dm: treewide: Use uclass_next_device_err when accessing second device Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 18/21] dm: blk: Do not use uclass_next_device_err Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 19/21] dm: treewide: Do not use the return value of simple uclass iterator Michal Suchanek
2022-09-25  8:28                                 ` [PATCH v4 20/21] dm: core: Switch uclass_*_device_err to use uclass_*_device_check Michal Suchanek
2022-09-25 11:08                                   ` [PATCH] fixup: " Michal Suchanek
2022-09-29 10:00                                   ` [PATCH v4 20/21] " Simon Glass
2022-09-25  8:28                                 ` [PATCH v4 21/21] dm: core: Do not stop uclass iteration on error Michal Suchanek
2022-09-27 21:37                                 ` [PATCH v5 00/15] " Michal Suchanek
2022-09-27 21:37                                   ` [PATCH v5 01/15] dm: core: Fix uclass_probe_all to really probe all devices Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:37                                   ` [PATCH v5 02/15] dm: treewide: Do not opencode uclass_probe_all() Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:37                                   ` [PATCH v5 03/15] dm: pci: Fix device PCI iteration Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:37                                   ` [PATCH v5 04/15] bootstd: Fix listing boot devices Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-10-02 19:19                                       ` Michal Suchánek
2022-09-27 21:37                                   ` [PATCH v5 05/15] usb: ether: Fix error handling in usb_ether_init Michal Suchanek
2022-09-27 21:37                                   ` [PATCH v5 06/15] stdio: Fix class iteration in stdio_add_devices() Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:37                                   ` [PATCH v5 07/15] video: ipuv3: Fix error handling when getting the display Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:38                                   ` [PATCH v5 08/15] w1: Fix bus counting in w1_get_bus Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:38                                   ` [PATCH v5 09/15] cmd: List all uclass devices regardless of probe error Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-10-02 19:10                                       ` Michal Suchánek
2022-10-03  1:10                                         ` Simon Glass
2022-09-27 21:38                                   ` [PATCH v5 10/15] dm: treewide: Use uclass_first_device_err when accessing one device Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:38                                   ` [PATCH v5 11/15] dm: treewide: Use uclass_next_device_err when accessing second device Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:38                                   ` [PATCH v5 12/15] dm: blk: Do not use uclass_next_device_err Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:38                                   ` [PATCH v5 13/15] dm: core: Switch uclass_*_device_err to use uclass_*_device_check Michal Suchanek
2022-09-27 21:38                                   ` [PATCH v5 14/15] dm: treewide: Do not use the return value of simple uclass iterator Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-27 21:38                                   ` [PATCH v5 15/15] dm: core: Do not stop uclass iteration on error Michal Suchanek
2022-09-29 10:00                                     ` Simon Glass
2022-09-29 10:00                                   ` [PATCH v5 00/15] " Simon Glass
2022-10-12 19:57                                   ` [PATCH v6 00/20] " Michal Suchanek
2022-10-12 19:57                                     ` [PATCH v6 01/20] dm: core: Fix uclass_probe_all to really probe all devices Michal Suchanek
2022-10-12 22:14                                       ` Simon Glass
2022-10-17 21:29                                       ` Simon Glass
2022-10-12 19:57                                     ` [PATCH v6 02/20] dm: treewide: Do not opencode uclass_probe_all() Michal Suchanek
2022-10-12 19:57                                     ` [PATCH v6 03/20] dm: pci: Fix device PCI iteration Michal Suchanek
2022-10-12 19:57                                     ` [PATCH v6 04/20] bootstd: Fix listing boot devices Michal Suchanek
2022-10-12 19:57                                     ` [PATCH v6 05/20] usb: ether: Fix error handling in usb_ether_init Michal Suchanek
2022-10-12 19:57                                     ` [PATCH v6 06/20] stdio: Fix class iteration in stdio_add_devices() Michal Suchanek
2022-10-12 19:57                                     ` [PATCH v6 07/20] video: ipuv3: Fix error handling when getting the display Michal Suchanek
2022-10-12 19:57                                     ` [PATCH v6 08/20] w1: Fix bus counting in w1_get_bus Michal Suchanek
2022-10-12 19:57                                     ` [PATCH v6 09/20] cmd: List all uclass devices regardless of probe error Michal Suchanek
2022-10-12 19:57                                     ` [PATCH v6 10/20] dm: treewide: Use uclass_first_device_err when accessing one device Michal Suchanek
2022-10-12 19:58                                     ` [PATCH v6 11/20] dm: treewide: Use uclass_next_device_err when accessing second device Michal Suchanek
2022-10-12 19:58                                     ` [PATCH v6 12/20] dm: blk: Do not use uclass_next_device_err Michal Suchanek
2022-10-12 19:58                                     ` [PATCH v6 13/20] net: eth-uclass: Do not set device on error Michal Suchanek
2022-10-12 19:58                                     ` [PATCH v6 14/20] dm: pci: Update error handling in pci_sriov_init Michal Suchanek
2022-10-12 22:14                                       ` Simon Glass
2022-10-12 19:58                                     ` [PATCH v6 15/20] mpc83xx: gazerbeam: Update sysinfo_get error handling Michal Suchanek
2022-10-12 22:14                                       ` Simon Glass
2022-10-17 21:29                                       ` Simon Glass
2022-10-12 19:58                                     ` [PATCH v6 16/20] dm: core: Switch uclass_foreach_dev_probe to use simple iterator Michal Suchanek
2022-10-12 22:14                                       ` Simon Glass
2022-10-17 21:29                                       ` Simon Glass
2022-10-12 19:58                                     ` [PATCH v6 17/20] dm: core: Switch uclass_*_device_err to use uclass_*_device_check Michal Suchanek
2022-10-12 19:58                                     ` [PATCH v6 18/20] dm: core: Non-activated device may be returned from uclass iterators that provide error handling Michal Suchanek
2022-10-12 22:14                                       ` Simon Glass
2022-10-12 19:58                                     ` [PATCH v6 19/20] dm: treewide: Do not use the return value of simple uclass iterator Michal Suchanek
2022-10-12 19:58                                     ` [PATCH v6 20/20] dm: core: Do not stop uclass iteration on error Michal Suchanek
2022-10-17 21:29                                     ` [PATCH v6 17/20] dm: core: Switch uclass_*_device_err to use uclass_*_device_check Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 13/20] net: eth-uclass: Do not set device on error Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 12/20] dm: blk: Do not use uclass_next_device_err Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 11/20] dm: treewide: Use uclass_next_device_err when accessing second device Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 10/20] dm: treewide: Use uclass_first_device_err when accessing one device Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 09/20] cmd: List all uclass devices regardless of probe error Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 07/20] video: ipuv3: Fix error handling when getting the display Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 08/20] w1: Fix bus counting in w1_get_bus Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 06/20] stdio: Fix class iteration in stdio_add_devices() Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 05/20] usb: ether: Fix error handling in usb_ether_init Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 04/20] bootstd: Fix listing boot devices Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 03/20] dm: pci: Fix device PCI iteration Simon Glass
2022-10-17 21:29                                     ` [PATCH v6 02/20] dm: treewide: Do not opencode uclass_probe_all() Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220830102303.GO28810@kitsune.suse.cz \
    --to=msuchanek@suse.de \
    --cc=marex@denx.de \
    --cc=morpheus.ibis@gmail.com \
    --cc=sjg@chromium.org \
    --cc=tmshlvck@gmail.com \
    --cc=u-boot@lists.denx.de \
    --cc=viktor.krivak@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.